Skip to content

Commit

Permalink
i#7044 sched deadlock: Fix theoretical deadlocks (#7087)
Browse files Browse the repository at this point in the history
Adds a check for the caller holding the previous input's lock before
acquiring it to retrieve the previous workload. The existing code
acquired the new lock without checking caller_holds_cur_input_lock,
which could hang (it happened not to because the only caller who sets
caller_holds_cur_input_lock to true set the current input to invalid and
set_cur_input() returns prior to this point).

Solves a 2nd theoretical lock issue: disallows the caller holding the input
lock if we might call add_to_ready_queue() (we disallow the lock holding
in dynamic mode in general to avoid this).  Adds an assert.

Issue: #7044
  • Loading branch information
derekbruening authored Nov 19, 2024
1 parent 336bac6 commit b158d1f
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
9 changes: 8 additions & 1 deletion clients/drcachesim/scheduler/scheduler_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2292,6 +2292,8 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::set_cur_input(
// straightforward).
if (options_.mapping == sched_type_t::MAP_TO_ANY_OUTPUT && prev_input != input &&
!inputs_[prev_input].at_eof) {
// The caller should never hold the input lock for MAP_TO_ANY_OUTPUT.
assert(!caller_holds_cur_input_lock);
add_to_ready_queue(output, &inputs_[prev_input]);
}
} else if (options_.schedule_record_ostream != nullptr &&
Expand All @@ -2313,7 +2315,12 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::set_cur_input(

int prev_workload = -1;
if (outputs_[output].prev_input >= 0 && outputs_[output].prev_input != input) {
std::lock_guard<mutex_dbg_owned> lock(*inputs_[outputs_[output].prev_input].lock);
// If the caller already holds the lock, do not re-acquire as that will hang.
auto scoped_lock =
(caller_holds_cur_input_lock && prev_input == outputs_[output].prev_input)
? std::unique_lock<mutex_dbg_owned>()
: std::unique_lock<mutex_dbg_owned>(
*inputs_[outputs_[output].prev_input].lock);
prev_workload = inputs_[outputs_[output].prev_input].workload;
}

Expand Down
3 changes: 2 additions & 1 deletion clients/drcachesim/scheduler/scheduler_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,8 @@ template <typename RecordType, typename ReaderType> class scheduler_impl_tmpl_t

// The caller cannot hold the output or input lock.
// The caller can hold the output's current input's lock but must pass
// true for the 3rd parameter in that case.
// true for the 3rd parameter in that case; holding the lock is not
// allowed for MAP_TO_ANY_OUTPUT.
stream_status_t
set_cur_input(output_ordinal_t output, input_ordinal_t input,
bool caller_holds_cur_input_lock = false);
Expand Down

0 comments on commit b158d1f

Please sign in to comment.