From 5d9a56d47a9cd5eefc4c8bf03709f2e96febb212 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Sun, 27 Sep 2026 20:25:26 -0300 Subject: 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 --- src/ninep/ctl.zig | 116 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 103 insertions(+), 13 deletions(-) (limited to 'src/ninep/ctl.zig') 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//; 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"); -- cgit v1.3