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

[Pavel Labath via libc-commits]

================
@@ -405,22 +405,55 @@ 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
+        // Clear the flag if we are the last one. The flag must be
         // cleared otherwise operations like trylock may fail even though
         // there is no competitors.
         if (guard.pending_count<role>() == 0)
           RwState::fetch_clear_pending_bit<role>(state,
                                                  cpp::MemoryOrder::RELAXED);
+        if constexpr (role == Role::Writer) {
+          int 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) {
+        // When a timeout triggers, the waiting thread wakes up, unregisters
----------------
labath wrote:

This is just a personal preference, but I find long comments like this  (it's now the most prominent part of this function)  more confusing than helpful. I don't think this is so subtle that it needs to be described in this length.
I might say something like:

> 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.

https://github.com/llvm/llvm-project/pull/201937