An Edit filter that can never be reaped no longer holds the Edit's answer: at the limit the command's group is killed and reaped off the answer's path, the stdin writer owning its input, so a filter writing its own ctl with > or >> answers EIO at 10 s instead of hanging; a running Edit keeps its own copies of the names it reports, and the panes it changes refuse Del, delete, rmdir, Undo, Redo, Get and Zerox (busy, EBUSY); e and r say unreadable files in Get's words; the reference says both sides of a filter's own ctl write need <>

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

5 files changed, 149 insertions, 27 deletions

diff --git a/src/builtins.zig b/src/builtins.zig
index 77e1169d..84904959 100644
--- a/src/builtins.zig
+++ b/src/builtins.zig
@@ -1141,6 +1141,7 @@ pub const Get = struct {
     pub const takes_arg = true;
     pub fn run(c: Ctx) void {
         const ctl = @import("ninep/ctl.zig");
+        if (@import("edit_cmd.zig").busyOn(c.p, c.id)) return c.p.reportFailure(c.id, "Get: " ++ @import("edit_cmd.zig").e_busy);
         const f = if (c.pane.file) |*file| file else return c.p.reportFailure(c.id, "Get: only a file pane takes it");
         const typed = std.mem.trim(u8, c.arg orelse "", " \t\r\n");
         // A directory pane: the directory read again (Dir.zig), as acme's
@@ -1247,6 +1248,7 @@ pub const Incl = struct {
 /// unsaved state, with a scroll and a cursor of its own (File.zerox).
 pub const Zerox = struct {
     pub fn run(c: Ctx) void {
+        if (@import("edit_cmd.zig").busyOn(c.p, c.id)) return c.p.reportFailure(c.id, "Zerox: " ++ @import("edit_cmd.zig").e_busy);
         const f = c.pane.file orelse return c.p.reportFailure(c.id, "Zerox: only a file pane takes it");
         if (f.output != null or f.listing != null or f.mini != null) return c.p.reportFailure(c.id, "Zerox: only a file pane takes it");
         const free = c.p.freeSlot() orelse return c.p.reportError(c.id, "Zerox", error.NoPaneSlots);
@@ -1302,6 +1304,7 @@ pub const Del = struct {
     pub const takes_arg = true;
     pub fn run(c: Ctx) void {
         const side = std.mem.trim(u8, c.arg orelse "", " \t\r\n");
+        if (@import("edit_cmd.zig").busyOn(c.p, c.id)) return c.p.reportFailure(c.id, "Del: " ++ @import("edit_cmd.zig").e_busy);
         // acme's Del asks winclean first (exec.c del): unsaved text is warned
         // about once, and the same Del again, nothing edited since, closes.
         if (warnModifiedIn(c, .Del, .{ .pane = c.id })) return;
@@ -1380,6 +1383,7 @@ pub const Edit = struct {
 /// limits.undo_max steps (256).
 pub const Undo = struct {
     pub fn run(c: Ctx) void {
+        if (@import("edit_cmd.zig").busyOn(c.p, c.id)) return c.p.reportFailure(c.id, "Undo: " ++ @import("edit_cmd.zig").e_busy);
         if (c.pane.file) |f| if (f.history.undo_len == 0) return c.p.setMessage(c.id, "Undo: nothing to undo");
         edit.doUndo(c.p, &c.pane.body);
     }
@@ -1388,6 +1392,7 @@ pub const Undo = struct {
 /// Redo the last undone edit.
 pub const Redo = struct {
     pub fn run(c: Ctx) void {
+        if (@import("edit_cmd.zig").busyOn(c.p, c.id)) return c.p.reportFailure(c.id, "Redo: " ++ @import("edit_cmd.zig").e_busy);
         if (c.pane.file) |f| if (f.history.redo_len == 0) return c.p.setMessage(c.id, "Redo: nothing to redo");
         edit.doRedo(c.p, &c.pane.body);
     }
diff --git a/src/edit_cmd.zig b/src/edit_cmd.zig
index ebeae223..83cc7e4f 100644
--- a/src/edit_cmd.zig
+++ b/src/edit_cmd.zig
@@ -67,6 +67,7 @@ pub const Write = struct {
 const World = struct {
     p: *Pardes,
     from: usize,
+    arena: std.mem.Allocator,
 
     fn check(ctx: *anyopaque, path: []const u8, why: *sam.Why) bool {
         const w: *World = @ptrCast(@alignCast(ctx));
@@ -103,7 +104,7 @@ const World = struct {
             const pane = slot orelse continue;
             const f = ninep_pane.fileOf(pane) orelse continue;
             if (!std.mem.eql(u8, f.path, path)) continue;
-            return fileOf(pane, id);
+            return fileOf(w.arena, pane, id) catch said(sam.File, why, "B: out of memory", .{});
         }
         return said(sam.File, why, "B: cannot open {s}", .{path});
     }
@@ -113,13 +114,9 @@ const World = struct {
         if (comptime pardes.hosted) if (filesystem.localPath(path)) |local| if (@import("exec.zig").isDirectory(local))
             return said([]const u8, why, "{s} is a directory", .{path});
         const bytes = filesystem.read(w.p, path) catch |err| {
-            const name = @errorName(err);
-            const words = if (std.mem.eql(u8, name, "FileNotFound"))
-                "file does not exist"
-            else if (std.mem.eql(u8, name, "AccessDenied") or std.mem.eql(u8, name, "PermissionDenied"))
-                "permission denied"
-            else
-                name;
+            // In the words Get says them (builtins.Get).
+            var buf: [128]u8 = undefined;
+            const words = if (err == error.FileNotFound) "no such file" else pardes.Messages.errorWords(err, &buf);
             return said([]const u8, why, "can't open {s}: {s}", .{ path, words });
         };
         defer w.p.gpa.free(bytes);
@@ -132,10 +129,13 @@ const World = struct {
     }
 };
 
-fn fileOf(pane: *panes.Pane, id: usize) sam.File {
+/// What the Edit sees of a pane. Its name is a copy in `arena`: the pane
+/// may be gone by the time a running Edit's end says it. Its text is the
+/// pane's own, read only while the Edit runs its commands' parse.
+fn fileOf(arena: std.mem.Allocator, pane: *panes.Pane, id: usize) !sam.File {
     const f = ninep_pane.fileOf(pane).?;
     return .{
-        .name = f.path,
+        .name = try arena.dupe(u8, f.path),
         .text = f.content,
         .dot = ninep_pane.dotOf(pane),
         .dirty = ninep_pane.dirtyOf(pane),
@@ -160,7 +160,7 @@ pub fn run(p: *Pardes, id: usize, command: []const u8) void {
     for (p.panes, 0..) |slot, i| {
         const q = slot orelse continue;
         if (q.file == null) continue;
-        files.append(arena, fileOf(q, i)) catch |err| return p.reportError(id, "Edit", err);
+        files.append(arena, fileOf(arena, q, i) catch |err| return p.reportError(id, "Edit", err)) catch |err| return p.reportError(id, "Edit", err);
     }
     std.mem.sort(sam.File, files.items, p, struct {
         fn before(core: *Pardes, a: sam.File, b: sam.File) bool {
@@ -171,7 +171,7 @@ pub fn run(p: *Pardes, id: usize, command: []const u8) void {
         if (f.id == id) break i;
     } else unreachable;
 
-    var world: World = .{ .p = p, .from = id };
+    var world: World = .{ .p = p, .from = id, .arena = arena };
     var why: sam.Why = .{};
     var text: [260]u8 = undefined;
     const res = sam.run(arena, files.items, cur, command, .{ .ctx = &world, .check = World.check, .open = World.open, .read = World.read, .refuseGet = World.refuseGet }, &why) catch |err| switch (err) {
@@ -219,7 +219,7 @@ pub fn run(p: *Pardes, id: usize, command: []const u8) void {
     for (res.jobs, 0..) |job, i| {
         const f = res.files[job.file];
         commands[i] = job.command;
-        cwds[i] = Pardes.paneDir(p.panes[f.id].?);
+        cwds[i] = arena.dupe(u8, Pardes.paneDir(p.panes[f.id].?)) catch |err| return p.reportError(id, "Edit", err);
         inputs[i] = .{ .bytes = if (job.c == '<') "" else arena.dupe(u8, f.text[job.q0..job.q1]) catch |err| return p.reportError(id, "Edit", err) };
     }
     p.pipe.seq +%= 1;
@@ -245,6 +245,29 @@ pub fn run(p: *Pardes, id: usize, command: []const u8) void {
     if (p.fs.serving) p.fs.edit_started = true;
 }
 
+/// What a pane whose Edit's commands are running refuses to do beside them:
+/// close, undo, redo, Get, Zerox (busyOn).
+pub const e_busy = "busy: an Edit's commands are running on it";
+
+/// Whether pane `id` (or a Zerox twin of it) is one the Edit whose commands
+/// are running will change or run a command on.
+pub fn busyOn(p: *Pardes, id: usize) bool {
+    const pd = p.pipe.edit_run orelse return false;
+    const pane = p.panes[id] orelse return false;
+    const twin = if (pane.file) |f| f.twin else 0;
+    for (pd.res.files, 0..) |f, i| {
+        const used = changes(f) or for (pd.res.jobs) |job| {
+            if (job.file == i) break true;
+        } else false;
+        if (!used) continue;
+        const q = p.panes[f.id] orelse continue;
+        if (q.serial != pd.serials[i]) continue;
+        if (f.id == id) return true;
+        if (twin != 0) if (q.file) |qf| if (qf.twin == twin) return true;
+    }
+    return false;
+}
+
 /// Whether the Edit changes file `f`: its text, its name, or its pane.
 fn changes(f: sam.File) bool {
     return f.ops.items.len > 0 or f.get != null or f.close or f.renamed;
@@ -727,3 +750,48 @@ test "an Edit with no command runs beside one whose commands run, unless it touc
     answer(p, &.{"a\nb\n"});
     try testing.expectEqualStrings("a\nb\n", p.panes[0].?.file.?.content);
 }
+
+test "a pane an Edit's commands will change is not closed under them, and one closed anyway is named, not read freed" {
+    const p = try th.withFile(testing.allocator, "b\na\n");
+    defer p.deinit();
+    const serial = th.serialOf(p);
+    const ctl = tree.Node.of(serial, .ctl);
+    try testing.expectEqual(tree.Status.ok, th.wr(p, ctl, "Edit ,|sleep 0.8; cat\n").reply.status);
+    // Del, acme's delete, rmdir, Undo, Redo, Get, Zerox: busy, EBUSY.
+    for ([_][]const u8{ "delete\n", "Del\n", "Undo\n", "Redo\n", "get\n", "Get\n", "Zerox\n" }) |line| {
+        const busy = th.wr(p, ctl, line);
+        try testing.expectEqual(tree.E.BUSY, busy.errno());
+        try testing.expect(std.mem.indexOf(u8, busy.reply.ename, e_busy) != null);
+    }
+    try testing.expectEqual(tree.E.BUSY, th.rmdir(p, tree.Node.of(serial, .dir)).errno());
+    try testing.expect(p.paneBySerial(serial) != null);
+    // Closed by a way that does not ask (a session's own teardown of one
+    // pane): the Edit's end names it from its own copy.
+    p.fs.edit_hold = .{ .asker = @ptrCast(p), .slot = 0, .seq = 1, .tag = 2, .node = ctl, .handle = 0, .written = 1 };
+    try p.removePane(0, null);
+    answer(p, &.{"a\nb\n"});
+    try testing.expectEqualStrings("Edit: /test.txt closed while its commands ran; nothing changed", filesystem.EditHold.answer(&p.fs).ename);
+}
+
+test "a session that ends while an Edit's commands run lets the Edit go" {
+    const p = try th.withFile(testing.allocator, "x\n");
+    try testing.expectEqual(tree.Status.ok, th.wr(p, tree.Node.of(th.serialOf(p), .ctl), "Edit ,|sleep 5; cat\n").reply.status);
+    try testing.expect(p.pipe.edit_run != null);
+    p.deinit(); // the testing allocator says if anything is kept
+}
+
+test "e and r say a file that cannot be read as Get says it" {
+    if (comptime !pardes.hosted or !filesystem.platform_has_fs) return error.SkipZigTest;
+    const p = try th.withFile(testing.allocator, "x\n");
+    defer p.deinit();
+    const ctl = tree.Node.of(th.serialOf(p), .ctl);
+    for ([_][2][]const u8{
+        .{ "Edit 0r /dev/zero\n", "not a regular file" },
+        .{ "Edit 0r /tmp\n", "is a directory" },
+        .{ "Edit 0r /nonexistent-pardes-file\n", "no such file" },
+    }) |c| {
+        const r = th.wr(p, ctl, c[0]);
+        try testing.expect(r.reply.status == .err);
+        try testing.expect(std.mem.indexOf(u8, r.reply.ename, c[1]) != null);
+    }
+}
diff --git a/src/ninep/ctl.zig b/src/ninep/ctl.zig
index c9ce41e1..105de07c 100644
--- a/src/ninep/ctl.zig
+++ b/src/ninep/ctl.zig
@@ -1003,6 +1003,7 @@ pub fn writePane(p: *Pardes, req: Req, pane: *Pane) Reply {
             }
             if (std.mem.eql(u8, line, "get")) {
                 if (!apply) continue;
+                if (@import("../edit_cmd.zig").busyOn(p, p.paneBySerial(serial).?)) return tree.failText(req.tag, E.BUSY, "get: " ++ @import("../edit_cmd.zig").e_busy);
                 // Quoting the line, as a builtin's refusal (Save's) does.
                 if (getRefused(p, pane, "get")) |said_in_ename| {
                     // refuse writes ename, where the words are: copy them out.
diff --git a/src/ninep/tree.zig b/src/ninep/tree.zig
index c78a1aa2..8d3c599f 100644
--- a/src/ninep/tree.zig
+++ b/src/ninep/tree.zig
@@ -937,6 +937,8 @@ fn remove(p: *Pardes, req: Req) Reply {
     };
     if (t.file != .dir) return Reply.fail(req.tag, E.PERM);
     const id = p.paneBySerial(t.serial) orelse return Reply.fail(req.tag, E.NOENT);
+    // A pane an Edit's running commands will change stays until they end.
+    if (@import("../edit_cmd.zig").busyOn(p, id)) return failText(req.tag, E.BUSY, @import("../edit_cmd.zig").e_busy);
     p.removePane(id, null) catch return Reply.fail(req.tag, E.IO);
     return .{ .tag = req.tag };
 }
diff --git a/src/selection_pipe.zig b/src/selection_pipe.zig
index 66444b04..c3e5e3a6 100644
--- a/src/selection_pipe.zig
+++ b/src/selection_pipe.zig
@@ -214,17 +214,53 @@ pub const Tasks = struct {
     }
 };
 
+/// The stdin writer's own: its copy of the input, freed by the writer when
+/// it is done, so a writer stuck on a command that never reads (and cannot
+/// be killed, below) can be let go of without its input dying under it.
 const WriterContext = struct {
+    gpa: std.mem.Allocator,
     io: std.Io,
     file: std.Io.File,
-    input: []const u8,
-    ok: bool = false,
+    input: []u8,
 };
 
 fn writeInput(context: *WriterContext) void {
-    defer context.file.close(context.io);
+    defer {
+        context.file.close(context.io);
+        context.gpa.free(context.input);
+        context.gpa.destroy(context);
+    }
     context.file.writeStreamingAll(context.io, context.input) catch return;
-    context.ok = true;
+}
+
+/// Kills the command's group, then `abandon`s it.
+fn abandonNow(io: std.Io, child: *std.process.Child, pid: std.posix.pid_t) void {
+    // Before the shell is reaped, while its group id cannot be another's.
+    std.posix.kill(-pid, .KILL) catch {};
+    std.posix.kill(pid, .KILL) catch {};
+    abandon(io, child.*);
+    child.id = null;
+    child.stdout = null;
+    child.stderr = null;
+}
+
+/// A command given up on, killed and reaped where nothing waits for it: a
+/// process in uninterruptible sleep (a write to a file of this very
+/// session, through its mount, waiting on the request whose answer is
+/// waiting on this command) dies only when that request is answered, and
+/// the answer must not wait on its death.
+fn abandon(io: std.Io, child: std.process.Child) void {
+    const Reap = struct {
+        fn run(i: std.Io, c: std.process.Child) void {
+            var reaped = c;
+            reaped.kill(i);
+        }
+    };
+    const thread = std.Thread.spawn(.{}, Reap.run, .{ io, child }) catch {
+        var c = child;
+        return c.kill(io);
+    };
+    thread.detach();
 }
 
 /// Run one POSIX shell command with exact stdin, concurrently draining stdout
@@ -350,23 +386,33 @@ pub fn runIn(
     }) catch return fail.k(.spawn);
     const pid = child.id.?;
 
-    var writer_context: WriterContext = .{
+    const writer_context = gpa.create(WriterContext) catch {
+        abandonNow(io, &child, pid);
+        return fail.k(.spawn);
+    };
+    writer_context.* = .{
+        .gpa = gpa,
         .io = io,
         .file = child.stdin.?,
-        .input = input,
+        .input = gpa.dupe(u8, input) catch {
+            gpa.destroy(writer_context);
+            abandonNow(io, &child, pid);
+            return fail.k(.spawn);
+        },
     };
     child.stdin = null; // writer_context owns and closes this endpoint
-    var writer: ?std.Thread = std.Thread.spawn(.{}, writeInput, .{&writer_context}) catch {
+    var writer: ?std.Thread = std.Thread.spawn(.{}, writeInput, .{writer_context}) catch {
         writer_context.file.close(io);
-        child.kill(io);
+        gpa.free(writer_context.input);
+        gpa.destroy(writer_context);
+        abandonNow(io, &child, pid);
         return fail.k(.spawn);
     };
-    // On every early return kill first, unblocking a command which never read
-    // stdin, then join the short-lived writer before its borrowed input dies.
-    defer if (writer) |thread| thread.join();
-    defer child.kill(io);
-    // Before the shell is reaped, while its group id cannot be another's.
-    defer if (child.id != null) std.posix.kill(-pid, .KILL) catch {};
+    // On every early return (a timeout, a ceiling) the answer goes at once:
+    // the group is killed and the shell left to be reaped elsewhere
+    // (abandon), and the writer, which owns its input, let go.
+    defer if (writer) |thread| thread.detach();
+    defer if (child.id != null) abandonNow(io, &child, pid);
     defer if (token != 0) Running.remove(pid);
     if (token != 0 and !Running.add(token, pid)) return fail.k(.signal);