A lock another open holds fails at once with file in use instead of parking

A contended lock parked until the holder unlocked, but through a kernel
or FUSE mount the kernel serialises writes to one file, so the parked
lock held up the holder's own unlock and close on that ctl, and the two
deadlocked. acme's qlock blocks; here the second lock is refused at once
with file in use (EBUSY) and the client retries, and nothing parks on
the lock any more.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

6 files changed, 34 insertions, 41 deletions

diff --git a/.agents/skills/pardes-9p/SKILL.md b/.agents/skills/pardes-9p/SKILL.md
index 0207f14c..6fea5838 100644
--- a/.agents/skills/pardes-9p/SKILL.md
+++ b/.agents/skills/pardes-9p/SKILL.md
@@ -146,7 +146,8 @@ it back evaluates it, and two clients addressing the same pane will interfere.
 a reserved zero, the dirty flag, the width in cells, the font and the tab
 width, then `current` or `notcurrent` — and takes `get` (reload from disk),
 `lock`/`unlock`, and any builtin that acts on a pane (`Del`, `Save f`,
-`Collapse`). Session builtins and settings go to the root `ctl`, which reads
+`Collapse`). A `lock` another open holds fails at once with `file in use`
+(EBUSY): retry it. Session builtins and settings go to the root `ctl`, which reads
 back every setting in the syntax it takes. A ctl write is checked whole
 first and refused as `unknown control message "X"` (EINVAL) and the like,
 a required argument missing included (`wrong #args ... "Mount"`); then a line
diff --git a/docs/fs.md b/docs/fs.md
index ac4e4b27..fee34941 100644
--- a/docs/fs.md
+++ b/docs/fs.md
@@ -219,8 +219,11 @@ whether the pane has the keyboard. It takes the pane's builtins (below),
 carries, and acme's `lock` and `unlock` (editors/acme/xfid.c:603-611), for an
 edit of several writes to `addr` and `data` that another client must not
 land in the middle of. As in acme the lock binds only the clients that take
-it: a `lock` while another open holds it waits until that open writes
-`unlock` or closes (or the pane does), and nothing else is refused for it --
+it: a `lock` while another open holds it fails at once with `file in use`
+(EBUSY), to be tried again, until that open writes `unlock` or closes (or
+the pane does) -- where acme's blocks, because through a kernel or FUSE
+mount a blocked write would hold up the holder's own `unlock` and close on
+that file -- and nothing else is refused for it --
 not a write to any other file, not the person at the keyboard. It belongs to
 the open that wrote it, so only that open's `unlock` is taken; a write on an
 open that cannot write (or the editor's own, on none) cannot lock. From a
diff --git a/src/ninep/ctl.zig b/src/ninep/ctl.zig
index 4afc82dc..5fe04f5c 100644
--- a/src/ninep/ctl.zig
+++ b/src/ninep/ctl.zig
@@ -374,11 +374,13 @@ pub fn readPane(p: *Pardes, req: Req, pane: *Pane) Reply {
 /// `lock` and `unlock` are acme's (editors/acme/xfid.c:603-611), so that a
 /// client can make an edit of several writes to addr and data without
 /// another's landing in between. As in acme the lock binds only the clients
-/// that ask for it: a `lock` while another open holds it waits (parked, as
-/// acme's qlock blocks the writer) until that open unlocks or closes; a
-/// write to any other file is never refused for it, nor is the person at
-/// the keyboard. It belongs to the open that wrote it, which alone may
-/// `unlock`, and closing that open or the pane gives it up.
+/// that ask for it; a write to any other file is never refused for it, nor
+/// is the person at the keyboard. It belongs to the open that wrote it,
+/// which alone may `unlock`, and closing that open or the pane gives it up.
+/// Where acme's qlock blocks a second locker, this refuses it at once with
+/// `file in use`, and the client tries again: through a kernel or FUSE
+/// mount a blocked write holds the file's writes, the holder's own `unlock`
+/// and close among them, so a waiting lock would never be let in.
 pub fn writePane(p: *Pardes, req: Req, pane: *Pane) Reply {
     // This open's handle is what holds the lock; a write that came on no
     // writable open (the editor's own) has none.
@@ -407,16 +409,13 @@ pub fn writePane(p: *Pardes, req: Req, pane: *Pane) Reply {
                 if (errno != 0) return Reply.fail(req.tag, errno);
             } else if (std.mem.eql(u8, line, "lock")) {
                 if (mine == null) return tree.failText(req.tag, E.INVAL, tree.e_bad_ctl);
-                if (other) return .{ .tag = req.tag, .status = .again };
+                if (other) return tree.failText(req.tag, E.BUSY, tree.e_in_use);
                 held = true;
                 if (apply) pane.fs.lock = mine;
             } else if (std.mem.eql(u8, line, "unlock")) {
                 if (!held) return tree.failText(req.tag, E.INVAL, e_not_locked);
                 held = false;
-                if (apply) {
-                    pane.fs.lock = null;
-                    pardes.turn.parked = true; // a `lock` waiting goes again
-                }
+                if (apply) pane.fs.lock = null;
             } else if (!apply) {
                 if (checkBuiltin(p, req, line, .pane)) |refusal| return refusal;
             } else if (runBuiltin(p, req, p.paneBySerial(serial).?, line)) |refusal| {
@@ -630,7 +629,7 @@ test "the root ctl reads the settings as a write takes them, and takes the sessi
 }
 
 
-test "a second lock waits until the holder unlocks or closes, and binds nobody else" {
+test "a second lock is refused until the holder unlocks or closes, and binds nobody else" {
     const p = try withFile(testing.allocator, "one\ntwo\n");
     defer p.deinit();
     const serial = serialOf(p);
@@ -649,37 +648,31 @@ test "a second lock waits until the holder unlocks or closes, and binds nobody e
 
     try testing.expectEqual(Status.ok, w.ctl(p, ctl_node, a, "lock\n").reply.status);
     try testing.expectEqual(Status.ok, w.ctl(p, ctl_node, a, "lock\n").reply.status);
-    // Another open's lock waits, and its unlock is refused; writes to the
-    // pane's other files are not.
-    try testing.expectEqual(Status.again, w.ctl(p, ctl_node, b, "lock\n").reply.status);
+    // Another open's lock is refused at once, and so is its unlock; writes
+    // to the pane's other files are not.
+    try testing.expectEqualStrings(tree.e_in_use, w.ctl(p, ctl_node, b, "lock\n").reply.ename);
+    try testing.expectEqual(E.BUSY, w.ctl(p, ctl_node, b, "lock\n").errno());
     try testing.expectEqualStrings(e_not_locked, w.ctl(p, ctl_node, b, "unlock\n").reply.ename);
     try testing.expectEqual(Status.ok, wr(p, Node.of(serial, .addr), "1").reply.status);
     try testing.expectEqual(Status.ok, wr(p, Node.of(serial, .data), "ONE\n").reply.status);
     try testing.expectEqualStrings("ONE\ntwo\n", p.panes[0].?.file.?.content);
 
-    // Unlocking lets the parked lock go again, and now it takes the lock.
-    pardes.turn.parked = false;
+    // Once the holder unlocks, the other's try takes it.
     try testing.expectEqual(Status.ok, w.ctl(p, ctl_node, a, "unlock\n").reply.status);
-    try testing.expect(pardes.turn.parked);
     try testing.expectEqual(Status.ok, w.ctl(p, ctl_node, b, "lock\n").reply.status);
-    try testing.expectEqual(Status.again, w.ctl(p, ctl_node, a, "lock\nunlock\n").reply.status);
+    try testing.expectEqual(E.BUSY, w.ctl(p, ctl_node, a, "lock\nunlock\n").errno());
     // Closing the holder gives it up.
-    pardes.turn.parked = false;
     _ = call(p, .{ .tag = 5, .op = .release, .node = ctl_node, .handle = b });
-    try testing.expect(pardes.turn.parked);
     try testing.expectEqual(Status.ok, w.ctl(p, ctl_node, a, "lock\nunlock\nlock\n").reply.status);
     try testing.expectEqual(E.INVAL, w.ctl(p, ctl_node, a, "unlock\nunlock\n").errno());
     try testing.expectEqual(@as(?u32, a), p.panes[0].?.fs.lock);
 
-    // The lock lives and dies with the pane: closing it wakes whoever waits.
+    // The lock lives and dies with the pane.
     const other = try th.newPane(p);
     const other_ctl = Node.of(other, .ctl);
     const c = call(p, .{ .tag = 6, .op = .open, .node = other_ctl, .omode = 1 }).reply.handle;
     try testing.expectEqual(Status.ok, w.ctl(p, other_ctl, c, "lock\n").reply.status);
-    pardes.turn.parked = false;
     try testing.expectEqual(Status.ok, th.rmdir(p, Node.of(other, .dir)).reply.status);
-    try testing.expect(pardes.turn.parked);
-    pardes.turn.parked = false;
     try testing.expectEqual(E.NOENT, w.ctl(p, other_ctl, c, "lock\n").errno());
     for ([_]u64{ ctl_node, other_ctl }, [_]u32{ a, c }) |node, h|
         _ = call(p, .{ .tag = 7, .op = .release, .node = node, .handle = h });
diff --git a/src/ninep/events.zig b/src/ninep/events.zig
index 34355e5c..172f5c00 100644
--- a/src/ninep/events.zig
+++ b/src/ninep/events.zig
@@ -116,8 +116,6 @@ pub fn noteRetire(p: *Pardes, pane: *Pane) void {
     tree.pty.shellGone(p, pane, false);
     p.fs.news = true; // a read held on its event or pty/data hears it went
     p.fs.listeners -|= pane.fs.readers;
-    // A write parked on its lock goes again, and finds the pane gone.
-    if (pane.fs.lock != null) pardes.turn.parked = true;
     if (pane.fs.unannounced) {
         pane.fs.unannounced = false;
         return;
diff --git a/src/ninep/tree.zig b/src/ninep/tree.zig
index a83b0193..f118e4b4 100644
--- a/src/ninep/tree.zig
+++ b/src/ninep/tree.zig
@@ -669,11 +669,9 @@ fn releaseHandle(p: *Pardes, req: Req) void {
             pn.fs.run = null;
         },
         // Closing the open that holds the lock gives it up, as acme's clunk
-        // of its ctlfid does (editors/acme/xfid.c:211); a write parked on
-        // `lock` goes again.
+        // of its ctlfid does (editors/acme/xfid.c:211).
         .ctl => if (pn.fs.lock == req.handle) {
             pn.fs.lock = null;
-            pardes.turn.parked = true;
         },
         .snapshot, .log => {},
     };
diff --git a/test/fs.py b/test/fs.py
index a6d24e0b..816f416c 100644
--- a/test/fs.py
+++ b/test/fs.py
@@ -218,21 +218,21 @@ def discovery(binary, embedded=False):
                 client.write('/exec', b'Msg woken\n')
                 reader.join(5)
                 assert woke and woke[0].endswith(b' woken\n'), woke
-            # A pane's ctl takes acme's lock: a second open's lock waits until
-            # the holder unlocks, and nothing else waits on it meanwhile.
+            # A pane's ctl takes acme's lock: a second open's lock fails at
+            # once with "file in use" until the holder unlocks, and nothing
+            # else waits on it meanwhile.
             with Client(address) as other:
                 mine = client.open(f'/pane/{first}/ctl', 2)
                 client.rpc(118, struct.pack('<IQI', mine, 0, 5) + b'lock\n')
                 theirs = other.open(f'/pane/{first}/ctl', 1)
-                locked = []
-                locker = threading.Thread(target=lambda: locked.append(other.rpc(118, struct.pack('<IQI', theirs, 0, 5) + b'lock\n')), daemon=True)
-                locker.start()
-                time.sleep(.2)
-                assert not locked
+                try:
+                    other.rpc(118, struct.pack('<IQI', theirs, 0, 5) + b'lock\n')
+                    raise AssertionError('a second lock was taken')
+                except OSError as refused:
+                    assert 'file in use' in str(refused), refused
                 client.write(f'/pane/{first}/addr', b'#0')
                 client.rpc(118, struct.pack('<IQI', mine, 0, 7) + b'unlock\n')
-                locker.join(5)
-                assert locked, 'a lock waiting on another open goes once it unlocks'
+                other.rpc(118, struct.pack('<IQI', theirs, 0, 5) + b'lock\n')
                 client.close(mine)
                 other.close(theirs)
             # /focus names the pane with the keyboard and moves it, as rio's