From 4987b62a90d1dea50e7c4cd13a02c7dfe2f9c2e3 Mon Sep 17 00:00:00 2001
From: Lukas Lalinsky <lukas@lalinsky.com>
Date: Sun, 9 Aug 2026 18:09:54 +0200
Subject: [PATCH] Fix two bugs in RwLock

Bug 1: `tryLock` can acquire a lock while another reader still holds
shared access to the lock. This one if fairly easy to reproduce, so the
test always catches it.

Bug 2: Cancelled writer can leave a stale semaphore permit, essentially
leaving the lock in an unusable state. This one is hard to reproduce
deterministically, the test catches it most of the time, but not always.

Let me know if you are OK with probabilistic tests like this, or I
should remove them.

Fixes https://codeberg.org/ziglang/zig/issues/36217
Fixes https://codeberg.org/ziglang/zig/issues/36178
---
 lib/std/Io/RwLock.zig | 92 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 89 insertions(+), 3 deletions(-)

diff --git a/lib/std/Io/RwLock.zig b/lib/std/Io/RwLock.zig
index de9bf86f366314f64ed45877c675dd2cbd9eee4a..def1997db95374977fece9dfb4b9283067f74bf6 100644
--- a/lib/std/Io/RwLock.zig
+++ b/lib/std/Io/RwLock.zig
@@ -27,10 +27,19 @@ const Count = @Int(.unsigned, @divFloor(@bitSizeOf(usize) - 1, 2));
 
 pub fn tryLock(rl: *RwLock, io: Io) bool {
     if (rl.mutex.tryLock()) {
+        // Unlike `lock`, this never registers in `writer_mask`, so holding the mutex does
+        // not stop a reader from taking the fast path; the CAS catches one that raced in
+        // after `state` was loaded.
         const state = @atomicLoad(usize, &rl.state, .seq_cst);
         if (state & reader_mask == 0) {
-            _ = @atomicRmw(usize, &rl.state, .Or, is_writing, .seq_cst);
-            return true;
+            _ = @cmpxchgStrong(
+                usize,
+                &rl.state,
+                state,
+                state | is_writing,
+                .seq_cst,
+                .seq_cst,
+            ) orelse return true;
         }
 
         rl.mutex.unlock(io);
@@ -61,7 +70,12 @@ pub fn lock(rl: *RwLock, io: Io) Io.Cancelable!void {
     if (state & reader_mask != 0)
         rl.semaphore.wait(io) catch |err| switch (err) {
             error.Canceled => {
-                rl.unlock(io);
+                // Clearing `is_writing` while still holding the mutex means the last reader
+                // either saw it set, and posts a permit only we can consume, or did not and
+                // never posts. A stale permit would let the next writer in past the readers.
+                const prev_state = @atomicRmw(usize, &rl.state, .And, ~is_writing, .seq_cst);
+                if (prev_state & reader_mask == 0) rl.semaphore.waitUncancelable(io);
+                rl.mutex.unlock(io);
                 return error.Canceled;
             },
         };
@@ -305,6 +319,78 @@ test "lock canceling" {
     try testing.expectEqual(rl, Io.RwLock.init);
 }
 
+test "tryLock does not race with readers" {
+    if (builtin.single_threaded) return error.SkipZigTest;
+
+    const Context = struct {
+        rl: Io.RwLock,
+
+        fn reader(ctx: *@This(), io: Io) !void {
+            while (true) {
+                if (ctx.rl.tryLockShared(io)) ctx.rl.unlockShared(io);
+                try io.checkCancel();
+            }
+        }
+    };
+
+    var ctx: Context = .{ .rl = .init };
+    const io = testing.io;
+
+    var future = io.concurrent(Context.reader, .{ &ctx, io }) catch |err| switch (err) {
+        error.ConcurrencyUnavailable => return error.SkipZigTest,
+    };
+    defer future.cancel(io) catch {};
+
+    for (0..1000) |_| {
+        if (!ctx.rl.tryLock(io)) continue;
+        defer ctx.rl.unlock(io);
+        const state = @atomicLoad(usize, &ctx.rl.state, .seq_cst);
+        try testing.expectEqual(0, state & reader_mask);
+    }
+
+    try testing.expectEqual(0, ctx.rl.semaphore.permits);
+}
+
+test "canceled writer does not leak a semaphore permit" {
+    if (builtin.single_threaded) return error.SkipZigTest;
+
+    const Writer = struct {
+        fn lockUnlock(rl: *Io.RwLock, io: Io) Io.Cancelable!void {
+            try rl.lock(io);
+            rl.unlock(io);
+        }
+    };
+
+    const io = testing.io;
+
+    var rl: Io.RwLock = .init;
+
+    for (0..1000) |_| {
+        rl.lockSharedUncancelable(io);
+
+        var wfuture = io.concurrent(Writer.lockUnlock, .{ &rl, io }) catch |err| switch (err) {
+            error.ConcurrencyUnavailable => {
+                rl.unlockShared(io);
+                return error.SkipZigTest;
+            },
+        };
+        var rfuture = io.concurrent(Io.RwLock.unlockShared, .{ &rl, io }) catch |err| switch (err) {
+            error.ConcurrencyUnavailable => {
+                rl.unlockShared(io);
+                wfuture.await(io) catch {};
+                return error.SkipZigTest;
+            },
+        };
+
+        // Races the last reader's `post` against the writer's cancelation.
+        wfuture.cancel(io) catch {};
+        rfuture.await(io);
+
+        try testing.expectEqual(0, rl.state);
+        try testing.expectEqual(0, rl.semaphore.permits);
+    }
+}
+
 fn semaphoreLockCancel(rl: *Io.RwLock, io: Io) !void {
     try rl.lock(io); //tests semaphore cancelling
 }
-- 
2.54.0