Skip to content
Snippets Groups Projects
Commit ad94d796 authored by Avi Kivity's avatar Avi Kivity
Browse files

sched: fix race in wake_lock()


wake_lock() prevents a race with the target thread waking up after wake_lock()
and attempting to take the mutex by itself, by changing the thread status
to sending_lock.

However, it doesn't prevent the reverse race:

t1                         t2
=========================  ======================
wait_for()
mutex::unlock()
thread::wait()

                           t1->wake()
                           mutex::lock()

mutex::lock() [A]
  thread::wait()

                           t1->wake_lock() [B]

After [A], t1 is waiting on the mutex, and after [B], it is waiting twice,
which is impossible and aborts on an assertion failure.

Fix by detecting that we're already waiting in send_lock() aborting the
send_lock().

Signed-off-by: default avatarAvi Kivity <avi@cloudius-systems.com>
parent 6c9712a0
No related branches found
No related tags found
No related merge requests found
...@@ -134,6 +134,33 @@ void mutex::send_lock(wait_record *wr) ...@@ -134,6 +134,33 @@ void mutex::send_lock(wait_record *wr)
} }
} }
// A variant of send_lock() with the following differences:
// - the mutex must be held by the caller
// - we do nothing if the thread we're sending the lock to is already
// waiting (and return false)
bool mutex::send_lock_unless_already_waiting(wait_record *wr)
{
trace_mutex_send_lock(this, wr);
assert(owned());
// count could not have been zero, so no need to test for it
count.fetch_add(1, std::memory_order_acquire);
for (auto& x : waitqueue) {
if (wr->thread() == x.thread()) {
return false;
}
}
// Queue the wait record, to be woken by an eventual unlock().
waitqueue.push(wr);
// No need for the Responsibility Hand-Off protocol, since we're holding
// the lock - no concurrent unlock can be happening.
return true;
}
// A thread waking up knowing it received a lock from send_lock() as part of // A thread waking up knowing it received a lock from send_lock() as part of
// a "wait morphing" protocol, must call receive_lock() to complete the lock's // a "wait morphing" protocol, must call receive_lock() to complete the lock's
// adoption by this thread. // adoption by this thread.
......
...@@ -755,8 +755,13 @@ void thread::wake_lock(mutex* mtx, wait_record* wr) ...@@ -755,8 +755,13 @@ void thread::wake_lock(mutex* mtx, wait_record* wr)
// let the thread acquire the lock itself // let the thread acquire the lock itself
return; return;
} }
st->lock_sent = true; // Send the lock to the thread, unless someone else already woke the us up,
mtx->send_lock(wr); // and we're sleeping in mutex::lock().
if (mtx->send_lock_unless_already_waiting(wr)) {
st->lock_sent = true;
} else {
st->st.store(status::waiting, std::memory_order_relaxed);
}
// since we're in status::sending_lock, no one can wake us except mutex::unlock // since we're in status::sending_lock, no one can wake us except mutex::unlock
} }
} }
......
...@@ -92,6 +92,7 @@ public: ...@@ -92,6 +92,7 @@ public:
// For wait morphing. Do not use unless you know what you are doing :-) // For wait morphing. Do not use unless you know what you are doing :-)
void send_lock(wait_record *wr); void send_lock(wait_record *wr);
bool send_lock_unless_already_waiting(wait_record *wr);
void receive_lock(); void receive_lock();
}; };
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment