[libc-commits] [libc] c36815b - [libc][rwlock] fix timeout writer signal stealing problem (#201937)

[via libc-commits]

Author: Schrodinger ZHU Yifan
Date: 2026-06-10T11:50:44-07:00
New Revision: c36815b3e2c42b70923ef6509532640c940dc00d

URL: https://github.com/llvm/llvm-project/commit/c36815b3e2c42b70923ef6509532640c940dc00d
DIFF: https://github.com/llvm/llvm-project/commit/c36815b3e2c42b70923ef6509532640c940dc00d.diff

LOG: [libc][rwlock] fix timeout writer signal stealing problem (#201937)

When a timeout triggers, the waiting thread wakes up, unregisters
itself from the waiting queue, and exits. However, if the timing-out
thread is preempted after waking up but before it can unregister,
and a concurrent unlock occurs during this window, the timing-out
thread may consume the wake-up signal.

For example, assume the lock is in writer-preference mode and a
writer (W0) holds the lock. A reader (R) and another writer (W1,
with a short timeout) arrive and join the queue. W1's timeout
expires, so it wakes up and attempts to acquire the queue lock,
but is preempted before succeeding. W0 then releases the lock and,
preferring writers, sends a wake-up signal to W1. When W1 resumes,
it acquires the queue lock, unregisters, and exits due to the
timeout, ignoring the wake-up signal. As a result, the reader (R)
is left waiting indefinitely, leading to a deadlock.

To fix this, we track whether the serialization number changed
specifically for writers, and propagate the wake signal if it did.
If the timing-out thread is a reader, signal consumption is safe
because:
1. If there are pending writers, they will be woken up first.
2. Otherwise, if there are pending readers, they are all woken up
   via broadcasting (notify_all), so one reader timing out does not
   steal others' signal.

Assisted-by: AI tools, manually checked

Added: 
    

Modified: 
    libc/src/__support/threads/raw_rwlock.h

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/threads/raw_rwlock.h b/libc/src/__support/threads/raw_rwlock.h
index 1b0bba9c72675..987cfa4f82152 100644
--- a/libc/src/__support/threads/raw_rwlock.h
+++ b/libc/src/__support/threads/raw_rwlock.h
@@ -375,7 +375,7 @@ class RawRwLock {
         return result;
 
       // Phase 5: register ourselves as a  reader.
-      int serial_number;
+      FutexWordType serial_number;
       {
         // The queue need to be protected by a mutex since the operations in
         // this block must be executed as a whole transaction. It is possible
@@ -405,22 +405,35 @@ class RawRwLock {
       }
 
       // Phase 7: unregister ourselves as a pending reader/writer.
+      bool writer_serial_changed = false;
       {
         // Similarly, the unregister operation should also be an atomic
         // transaction.
         WaitingQueue::Guard guard = queue.acquire(is_pshared);
         guard.pending_count<role>()--;
-        // Clear the flag if we are the last reader. The flag must be
-        // cleared otherwise operations like trylock may fail even though
-        // there is no competitors.
+        // Clear the flag if we are the last pending thread of this role. The
+        // flag must be cleared; otherwise operations like trylock may fail even
+        // though there are no competitors.
         if (guard.pending_count<role>() == 0)
           RwState::fetch_clear_pending_bit<role>(state,
                                                  cpp::MemoryOrder::RELAXED);
+        if constexpr (role == Role::Writer) {
+          FutexWordType new_serial =
+              guard.serialization<role>().load(cpp::MemoryOrder::RELAXED);
+          writer_serial_changed = new_serial != serial_number;
+        }
       }
 
-      // Phase 8: exit the loop is timeout is reached.
-      if (timeout_flag)
+      // Phase 8: exit the loop if timeout is reached.
+      if (timeout_flag) {
+        // A timed out (but not unregistered) thread can still be the target of
+        // a wakeup signal. If this happens, we must wake up the next thread to
+        // avoid a deadlock. This is only a problem for writers because readers
+        // are woken up all at once and we prefer waking up writers first.
+        if (writer_serial_changed)
+          notify_pending_threads();
         return LockResult::TimedOut;
+      }
 
       // Phase 9: reload the state and retry the acquisition.
       old = RwState::spin_reload<role>(state, get_preference(), spin_count);
@@ -432,6 +445,14 @@ class RawRwLock {
   // Since notifcation routine is colder we mark it as noinline explicitly.
   [[gnu::noinline]]
   LIBC_INLINE void notify_pending_threads() {
+    // We wake up writers first. This ordering is relevant for the correctness
+    // of timeout handling in lock_slow. Because writers are preferred, we do
+    // not need to handle reader timeouts specially:
+    // 1. If there are pending writers, they are woken up first, so a reader
+    //    cannot steal their wake-up signal.
+    // 2. If there are only pending readers, they are all woken up together
+    //    (via notify_all), so a timed-out reader cannot steal other readers'
+    //    wake-up signals.
     enum class WakeTarget { Readers, Writers, None };
     WakeTarget status;