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

1 files changed, 17 insertions, 24 deletions

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 });