A pane's ctl takes acme's lock and unlock

A client doing an edit of several writes to addr and data had no way to
keep another client's from landing in between. acme's window ctl takes
lock and unlock for this (editors/acme/xfid.c:603-611): a qlock that
blocks a second locker, owned by the fid that wrote it and given up when
that fid is clunked, binding only clients that ask. pardes does the same:
the open's record holds it, a second lock parks until unlock, close or
the pane closing, and no other write is refused for it.

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

4 files changed, 122 insertions, 15 deletions

diff --git a/src/ninep/ctl.zig b/src/ninep/ctl.zig
index 7625f501..83b1196f 100644
--- a/src/ninep/ctl.zig
+++ b/src/ninep/ctl.zig
@@ -2,8 +2,8 @@
 //! a right click on it and writing one to `exec` a middle click, at the
 //! active pane from the root and at that pane from /pane/<n>/; reading either
 //! answers the serials the last command made or touched. /status reports the
-//! editor, and a pane's ctl its acme status line and the one verb, `get`,
-//! that no file of its own would say any better.
+//! editor, and a pane's ctl its acme status line and the verbs no file of its
+//! own would say any better: `get`, `lock` and `unlock`.
 const std = @import("std");
 const pardes = @import("../pardes.zig");
 const panes = @import("../panes.zig");
@@ -179,17 +179,48 @@ pub fn readPane(p: *Pardes, req: Req, pane: *Pane) Reply {
     return tree.stagedReply(p, req);
 }
 
-/// `get` is the one thing here that no file of the pane's own would say: it
-/// reloads the buffer from the name it carries, wherever that name resolves.
-/// Repeating it in one write would only reload the same bytes, so it runs once.
+/// `get` reloads the buffer from the name it carries, wherever that name
+/// resolves. Repeating it in one write would only reload the same bytes, so
+/// it runs once.
+///
+/// `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.
 pub fn writePane(p: *Pardes, req: Req, pane: *Pane) Reply {
+    // The record of this open is what holds the lock; a write that came on
+    // no writable open (the editor's own) has none.
+    const mine: ?u8 = if (tree.openOf(p, req)) |o| @intCast(o - &p.fs.opens[0]) else null;
+    const other = pane.fs.lock != null and pane.fs.lock != mine;
     var asked = false;
-    var it = std.mem.splitScalar(u8, req.data, '\n');
-    while (it.next()) |raw| {
-        const line = std.mem.trim(u8, raw, " \t\r");
-        if (line.len == 0) continue;
-        if (!std.mem.eql(u8, line, "get")) return tree.failText(req.tag, E.INVAL, tree.e_bad_ctl);
-        asked = true;
+    // Checked whole before anything applies, so a write that must wait for
+    // the lock has done nothing yet when it goes again.
+    for ([2]bool{ false, true }) |apply| {
+        var held = !other and pane.fs.lock != null;
+        var it = std.mem.splitScalar(u8, req.data, '\n');
+        while (it.next()) |raw| {
+            const line = std.mem.trim(u8, raw, " \t\r");
+            if (line.len == 0) continue;
+            if (std.mem.eql(u8, line, "get")) {
+                asked = true;
+            } 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 };
+                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
+                }
+            } else return tree.failText(req.tag, E.INVAL, tree.e_bad_ctl);
+        }
     }
     if (asked) {
         const errno = get(p, pane);
@@ -198,6 +229,8 @@ pub fn writePane(p: *Pardes, req: Req, pane: *Pane) Reply {
     return .{ .tag = req.tag, .written = @intCast(req.data.len) };
 }
 
+const e_not_locked = "window not locked by this open";
+
 fn get(p: *Pardes, pane: *Pane) u16 {
     const f = pane_files.fileOf(pane) orelse return 0;
     if (!panes.Output.fileTraits(f.output).saves) return 0;
@@ -251,13 +284,13 @@ test "pane ctl read is index's five fields plus width in cells, font and tab wid
     try testing.expectEqualStrings("'it''s'", w.buffered());
 }
 
-test "the pane ctl takes get, and nothing that a file of its own now answers" {
+test "the pane ctl takes get, lock and unlock, and nothing that a file of its own now answers" {
     const gpa = testing.allocator;
     const p = try withFile(gpa, "one\ntwo\n");
     defer p.deinit();
     const ctl_node = Node.of(serialOf(p), .ctl);
     for ([_][]const u8{
-        "menu",       "nomenu",     "dump echo hi", "font Go Mono", "lock",   "bogus",     "DEL",
+        "menu",       "nomenu",     "dump echo hi", "font Go Mono", "lock x", "bogus",     "DEL",
         "name x.txt", "put",        "del",          "delete",       "Look x", "Exec Save", "clean",
         "dirty",      "cleartag",   "dot=addr",     "addr=dot",     "show",   "mark",      "nomark",
         "scroll",     "limit=addr", "get x",        "look /tmp",    "exec Del",
@@ -265,6 +298,63 @@ test "the pane ctl takes get, and nothing that a file of its own now answers" {
     try testing.expect(p.paneBySerial(serialOf(p)) != null);
 }
 
+test "a second lock waits 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);
+    const ctl_node = Node.of(serial, .ctl);
+    const w = struct {
+        fn ctl(pp: *Pardes, node: u64, h: u32, data: []const u8) th.Answer {
+            return call(pp, .{ .tag = 4, .op = .write, .node = node, .handle = h, .data = data });
+        }
+    };
+    const a = call(p, .{ .tag = 1, .op = .open, .node = ctl_node, .omode = 2 }).reply.handle;
+    const b = call(p, .{ .tag = 2, .op = .open, .node = ctl_node, .omode = 1 }).reply.handle;
+    try testing.expect(a != 0 and b != 0 and a != b);
+    // Reading the status line holds nothing, so it cannot lock either.
+    try testing.expectEqual(@as(u32, 0), call(p, .{ .tag = 3, .op = .open, .node = ctl_node }).reply.handle);
+    try testing.expectEqual(E.INVAL, wr(p, ctl_node, "lock\n").errno());
+
+    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);
+    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;
+    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);
+    // 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(?u8, @intCast(a - 1)), p.panes[0].?.fs.lock);
+
+    // The lock lives and dies with the pane: closing it wakes whoever waits.
+    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 });
+    try testing.expect(p.panes[0].?.fs.lock == null);
+    for (p.fs.opens) |o| try testing.expect(o.node == 0);
+}
+
 test "exec runs a builtin at the pane and records the pane it acted on" {
     const gpa = testing.allocator;
     const p = try withFile(gpa, "Msg fs-ran\n");
diff --git a/src/ninep/events.zig b/src/ninep/events.zig
index 8f66cf3a..cb31e2aa 100644
--- a/src/ninep/events.zig
+++ b/src/ninep/events.zig
@@ -121,6 +121,8 @@ pub fn noteRetire(p: *Pardes, pane: *Pane) void {
     tree.pty.shellGone(p, pane);
     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/pane.zig b/src/ninep/pane.zig
index c03cd776..3d7834c4 100644
--- a/src/ninep/pane.zig
+++ b/src/ninep/pane.zig
@@ -46,6 +46,8 @@ pub const State = struct {
     /// The open record (tree.zig) of the pty/run waiting on this shell's
     /// current command.
     run: ?u8 = null,
+    /// The open record of the ctl open that wrote `lock` (ctl.zig).
+    lock: ?u8 = null,
     /// The host started a shell it could not teach to mark its prompts.
     unmarked: bool = false,
     /// Installed during this update; /log hears about it once the update ends
diff --git a/src/ninep/tree.zig b/src/ninep/tree.zig
index bc32ad33..c9e7b31a 100644
--- a/src/ninep/tree.zig
+++ b/src/ninep/tree.zig
@@ -122,6 +122,8 @@ pub const Open = struct {
         event: bool,
         /// The one open reading a pane's pty/data.
         pty_data,
+        /// An open that may write a pane's ctl, and so hold its lock.
+        ctl,
     } = .{ .snapshot = null },
     /// A read that found nothing yet (`.again`), kept to be answered when
     /// this open's file has something, the way factotum keeps its log's
@@ -141,7 +143,7 @@ pub const Open = struct {
             .snapshot => |bytes| if (bytes) |b| gpa.free(b),
             .log => |log| gpa.free(log.bytes),
             .run => |run| gpa.free(run.output),
-            .event, .pty_data => {},
+            .event, .pty_data, .ctl => {},
         }
         o.* = .{};
     }
@@ -577,6 +579,10 @@ fn open(p: *Pardes, req: Req, target: Target) Reply {
                 .event => if (reader and pn.fs.event_reader) return failText(req.tag, E.BUSY, e_in_use) else .{ .event = reader },
                 .pty_data => if (!reader) return .{ .tag = req.tag } else if (pn.fs.pty_reader) return failText(req.tag, E.BUSY, e_in_use) else .pty_data,
                 .pty_run => .{ .run = .{} },
+                .ctl => switch (req.omode & 3) {
+                    1, 2 => .ctl, // OWRITE, ORDWR
+                    else => return .{ .tag = req.tag },
+                },
                 else => return .{ .tag = req.tag },
             };
         },
@@ -605,7 +611,7 @@ fn open(p: *Pardes, req: Req, target: Target) Reply {
             pn.fs.event_reader = pn.fs.event_reader or reader;
         },
         .pty_data => pane_of.?.fs.pty_reader = true,
-        .run => {},
+        .run, .ctl => {},
     }
     return .{ .tag = req.tag, .handle = @intCast(i + 1) };
 }
@@ -652,6 +658,13 @@ fn releaseHandle(p: *Pardes, req: Req) void {
         .run => if (pn.fs.run == @as(u8, @intCast(req.handle - 1))) {
             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.
+        .ctl => if (pn.fs.lock == @as(u8, @intCast(req.handle - 1))) {
+            pn.fs.lock = null;
+            pardes.turn.parked = true;
+        },
         .snapshot, .log => {},
     };
     o.deinit(p.gpa);