diff options
| -rw-r--r-- | .agents/skills/pardes-9p/SKILL.md | 3 | ||||
| -rw-r--r-- | docs/fs.md | 7 | ||||
| -rw-r--r-- | src/ninep/ctl.zig | 41 | ||||
| -rw-r--r-- | src/ninep/events.zig | 2 | ||||
| -rw-r--r-- | src/ninep/tree.zig | 4 | ||||
| -rw-r--r-- | test/fs.py | 18 |
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 @@ -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 => {}, }; @@ -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 |