diff options
| -rw-r--r-- | docs/fs.md | 4 | ||||
| -rw-r--r-- | src/detached/server.zig | 18 | ||||
| -rw-r--r-- | src/gui/gui.zig | 14 | ||||
| -rw-r--r-- | src/macos.zig | 8 | ||||
| -rw-r--r-- | src/ninep/ctl.zig | 16 | ||||
| -rw-r--r-- | src/ninep/pty.zig | 8 | ||||
| -rw-r--r-- | src/pardes.zig | 7 | ||||
| -rw-r--r-- | src/tty/tty.zig | 10 | ||||
| -rw-r--r-- | test/fs.py | 40 |
9 files changed, 103 insertions, 22 deletions
@@ -958,7 +958,9 @@ its directory: one that is gone is refused before anything runs, `exec: there, not executable, a script whose interpreter is not there) fails the write with why -- `shell: shell not found`, or `shell: interpreter /no/such/interp not found` for a script whose `#!` names a program that is -not there, ENOENT -- keeping the terminal; a `Tty` naming such a shell or +not there, ENOENT -- keeping the terminal and its running shell: a shell +not there is refused before anything runs, and the host starts the new one +before the old goes, so one that cannot start leaves the old be; a `Tty` naming such a shell or script is refused before anything runs (`Tty: interpreter ... not found`, its `err` the only record), and one whose shell cannot start fails the same way and leaves no pane. The host knows before it answers: the child reports a failed exec diff --git a/src/detached/server.zig b/src/detached/server.zig index a08a0342..676b5b95 100644 --- a/src/detached/server.zig +++ b/src/detached/server.zig @@ -485,9 +485,11 @@ pub const Session = struct { fn spawn(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void { const s = of(ctx); if (pane >= s.ptys.len) return; // the core indexes its own panes - s.closePty(pane); s.harvest(); - if (s.ptys[pane].pid != 0) return s.core.reportError(pane, "shell", error.ShellClosing); + // The new shell starts before a running one goes: one that cannot + // start leaves the running one be (pty/ctl's exec). + const running = s.ptys[pane].fd >= 0; + if (!running and s.ptys[pane].pid != 0) return s.core.reportError(pane, "shell", error.ShellClosing); for (s.retired_shells) |shell| { if (shell.pid == 0) break; } else return s.core.reportError(pane, "shell", error.ShellClosing); @@ -500,7 +502,17 @@ pub const Session = struct { s.core.screen_h, s.core.screen_w, s.ninep, - ) catch |err| return s.core.shellFailed(pane, err); + ) catch |err| { + if (running) return s.core.restartFailed(pane, err); + return s.core.shellFailed(pane, err); + }; + s.closePty(pane); + s.harvest(); + if (s.ptys[pane].pid != 0) { + _ = libc.close(child.file.handle); + host_io.retireShell(child.pid); + return s.core.reportError(pane, "shell", error.ShellClosing); + } const command = if (s.core.panes[pane]) |pn| pn.command != null else false; s.ptys[pane] = .{ .fd = child.file.handle, .pid = child.pid, .cmd = .{ .watched = command and host_io.watchExit(child.pid), .fd = child.file.handle } }; setNonblock(child.file.handle); diff --git a/src/gui/gui.zig b/src/gui/gui.zig index cb3bdd97..851218bb 100644 --- a/src/gui/gui.zig +++ b/src/gui/gui.zig @@ -4553,10 +4553,20 @@ fn gridPostPresent(ctx: ?*anyopaque) void { fn spawnPane(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void { const s = shellOf(ctx); s.reap(); + // The new shell starts before a running one goes: one that cannot + // start leaves the running one be (pty/ctl's exec). + const running = s.ptys[pane] != null; + const child = host_io.forkShell(s.core, pane, s.prompt_rcs, s.core.shellBin(), cwd, s.core.screen_h, s.core.screen_w, s.fs) catch |err| { + if (running) return s.core.restartFailed(pane, err); + return s.core.shellFailed(pane, err); + }; s.closePty(pane); - if (s.ptys[pane] != null) return s.core.reportError(pane, "shell", error.WorkersBusy); + if (s.ptys[pane] != null) { + _ = libc.close(child.file.handle); + host_io.retireShell(child.pid); + return s.core.reportError(pane, "shell", error.WorkersBusy); + } s.gens[pane] +%= 1; - const child = host_io.forkShell(s.core, pane, s.prompt_rcs, s.core.shellBin(), cwd, s.core.screen_h, s.core.screen_w, s.fs) catch |err| return s.core.shellFailed(pane, err); const command = s.core.panes[pane].?.command != null; const pt: Pty = .{ .fd = child.file.handle, .pid = child.pid, .serial = s.core.panes[pane].?.serial, .cmd = .{ .watched = command and host_io.watchExit(child.pid), .fd = child.file.handle } }; s.ptys[pane] = pt; diff --git a/src/macos.zig b/src/macos.zig index 9b17803b..e839816c 100644 --- a/src/macos.zig +++ b/src/macos.zig @@ -2437,11 +2437,17 @@ fn hostState(ctx: ?*anyopaque) *State { fn spawnShell(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void { const st = hostState(ctx); const core = st.core; + // The new shell starts before a running one goes: one that cannot + // start leaves the running one be (pty/ctl's exec). + const running = st.ptys[pane] != null; + const child = host_io.forkShell(core, pane, &st.prompt_rcs, core.shellBin(), cwd, core.screen_h, core.screen_w, st.ninep) catch |err| { + if (running) return core.restartFailed(pane, err); + return core.shellFailed(pane, err); + }; reap(st, pane); st.gens[pane] +%= 1; const gen = st.gens[pane]; - const child = host_io.forkShell(core, pane, &st.prompt_rcs, core.shellBin(), cwd, core.screen_h, core.screen_w, st.ninep) catch |err| return core.shellFailed(pane, err); st.ptys[pane] = .{ .file = child.file, .pid = child.pid, diff --git a/src/ninep/ctl.zig b/src/ninep/ctl.zig index 31382279..92342525 100644 --- a/src/ninep/ctl.zig +++ b/src/ninep/ctl.zig @@ -1996,7 +1996,7 @@ test "pty/ctl exec in a directory that is gone fails ENOENT; a shell that cannot try testing.expectEqualStrings("shell: access denied", p.fs.late_failure[0..p.fs.late_failure_len]); } -test "a script whose interpreter is not there: Tty refuses it up front, only an err logged" { +test "a script whose interpreter is not there: Tty refuses it up front, only an err logged, and pty/ctl exec keeps the running shell" { const p = try th.withTerm(testing.allocator); defer p.deinit(); const Starting = struct { @@ -2023,6 +2023,20 @@ test "a script whose interpreter is not there: Tty refuses it up front, only an try testing.expect(th.logHas(p, "Tty: interpreter /no/such/interp not found\n")); try testing.expect(!th.logHas(p, "\nmsg ")); try testing.expect(!th.logHas(p, "\ndel ")); + // The shell to start again is that script: refused before the running + // one goes. + const id = p.paneBySerial(serial).?; + p.panes[id].?.shell = try p.gpa.dupe(u8, try std.fmt.bufPrint(&line, "{s}/bad", .{dir})); + p.setCwd(id, dir); + const exec_refused = wr(p, Node.of(serial, .pty_ctl), "exec\n"); + try testing.expectEqual(E.NOENT, exec_refused.errno()); + try testing.expectEqualStrings("exec: interpreter /no/such/interp not found", exec_refused.reply.ename); + // One the host found could not start (the script changed after): said + // with the interpreter's name, and the running shell is not gone. + p.fs.late_failure_len = 0; + p.restartFailed(@intCast(id), error.InterpreterNotFound); + try testing.expectEqualStrings("shell: interpreter /no/such/interp not found", p.fs.late_failure[0..p.fs.late_failure_len]); + try testing.expect(!p.panes[id].?.shell_failed); } test "every EINVAL a write gets says why, in its err record too; DEL is a control character in a line" { diff --git a/src/ninep/pty.zig b/src/ninep/pty.zig index aa0f499a..bf5f432c 100644 --- a/src/ninep/pty.zig +++ b/src/ninep/pty.zig @@ -67,6 +67,14 @@ pub fn writeCtl(p: *Pardes, req: Req, id: usize) Reply { const dir = pane.cwdSlice(); return tree.failText(req.tag, E.NOENT, std.fmt.bufPrint(&p.fs.ename, "exec: {s}: no such directory", .{dir[0..@min(dir.len, 256)]}) catch "exec: no such directory"); }; + // Nor is a shell that is not there, or a script whose + // interpreter is not: refused, and the running shell kept. + if (comptime pardes.hosted) if (!apply and std.mem.eql(u8, line, "exec")) if (p.panes[id]) |pane| { + const bin = pane.shell orelse p.shellBin(); + var why: [320]u8 = undefined; + if (@import("../host_io.zig").Shell.refusal(bin[0..@min(bin.len, 200)], &why)) |refused| + return tree.failText(req.tag, E.NOENT, std.fmt.bufPrint(&p.fs.ename, "exec: {s}", .{refused}) catch "exec: no such shell"); + }; if (!verb(p, id, line, apply)) return tree.failText(req.tag, E.INVAL, if (outOfRange(line)) e_winsize_range else e_bad_pty_ctl); } } diff --git a/src/pardes.zig b/src/pardes.zig index d2fd795b..76ec2176 100644 --- a/src/pardes.zig +++ b/src/pardes.zig @@ -5353,6 +5353,13 @@ pub const Pardes = struct { } } + /// A shell started again in a terminal whose shell runs (pty/ctl's + /// exec) did not start: the host kept the running one, so only said. + pub fn restartFailed(p: *Pardes, id: u8, err: anyerror) void { + if (p.panes[id] == null) return; + p.sayShellFailure(id, err); + } + fn sayShellFailure(p: *Pardes, id: u8, err: anyerror) void { const pane = p.panes[id] orelse return; if (err == error.FileNotFound or err == error.NotDir) { diff --git a/src/tty/tty.zig b/src/tty/tty.zig index 3c8c90bc..a6f6bdfc 100644 --- a/src/tty/tty.zig +++ b/src/tty/tty.zig @@ -1253,9 +1253,15 @@ const Shell = struct { fn spawn(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void { const s = of(ctx); - closePty(ctx, pane); // a shell still in the slot goes first, reaped + // The new shell starts before a running one goes: one that cannot + // start leaves the running one be (pty/ctl's exec). + const running = s.ptys[pane] != null; + const child = host_io.forkShell(s.core, pane, s.prompt_rcs, s.core.shellBin(), cwd, s.core.screen_h, s.core.screen_w, s.fs) catch |err| { + if (running) return s.core.restartFailed(pane, err); + return s.core.shellFailed(pane, err); + }; + closePty(ctx, pane); // a shell still in the slot goes, reaped s.gens[pane] +%= 1; - const child = host_io.forkShell(s.core, pane, s.prompt_rcs, s.core.shellBin(), cwd, s.core.screen_h, s.core.screen_w, s.fs) catch |err| return s.core.shellFailed(pane, err); const command = if (s.core.panes[pane]) |pn| pn.command != null else false; s.ptys[pane] = .{ .file = child.file, .pid = child.pid, .reader = .{ .any_future = null, .result = {} }, .cmd = .{ .watched = command and host_io.watchExit(child.pid), .fd = child.file.handle } }; var lbuf: [pardes.memory.limits.host_path_cap + 1]u8 = undefined; @@ -442,6 +442,27 @@ def run_file(binary): assert run(client, term, b'true\n') == b'exit 0\n' assert client.read(f'/pane/{term}/pty/ctl').split()[2] == b'2', client.read(f'/pane/{term}/pty/ctl') client.write(f'/pane/{term}/pty/ctl', b'winsize 80 24\n') + # A shell started again that cannot start (its directory is + # there but cannot be entered) fails the write and leaves the + # running shell be: runs still work. + locked = root / 'locked' + locked.mkdir() + assert run(client, term, f'cd {locked}\n'.encode()) == b'exit 0\n' + deadline = time.monotonic() + 5 + while str(locked).encode() not in client.read('/index'): + assert time.monotonic() < deadline, client.read('/index') + time.sleep(.05) + locked.chmod(0o600) + try: + client.write(f'/pane/{term}/pty/ctl', b'exec\n') + raise AssertionError('pty/ctl exec of a shell that cannot start was taken') + except OSError as refused: + assert 'shell' in str(refused), refused + finally: + locked.chmod(0o755) + kept = run(client, term, b'true\n') + assert kept == b'exit 0\n', (kept, client.read('/log')[-600:]) + assert run(client, term, f'cd {root}\n'.encode()) == b'exit 0\n' with Client(address) as other: slow = [] waiter = threading.Thread(target=lambda: slow.append(run(other, term, b'sleep 1\n')), daemon=True) @@ -522,20 +543,15 @@ def new_terminals_named_once(binary): bad.write_bytes(b'#!/nonexistent/interp\n') bad.chmod(0o755) panes = client.read('/index') - try: - client.write('/pane/1/ctl', f'Tty {bad}\n'.encode()) - raise AssertionError('Tty of a shell that cannot start was taken') - except OSError as refused: - assert 'shell' in str(refused), refused + for where, line in (('/pane/1/ctl', f'Tty {bad}\n'), ('/ctl', f'Shell {bad}\n')): + try: + client.write(where, line.encode()) + raise AssertionError(f'{line} of a shell that cannot start was taken') + except OSError as refused: + assert 'interpreter /nonexistent/interp not found' in str(refused), refused time.sleep(.3) assert client.read('/index').count(b'\n') == panes.count(b'\n'), client.read('/index') - client.write('/ctl', f'Shell {bad}\n'.encode()) - try: - client.write(f'/pane/{made[0]}/pty/ctl', b'exec\n') - raise AssertionError('pty/ctl exec of a shell that cannot start was taken') - except OSError as refused: - assert 'shell' in str(refused), refused - client.write('/ctl', b'Shell\n') + assert b'shell: shell not found' not in client.read('/log'), client.read('/log') assert str(made[0]).encode() in b' '.join(r.split()[0] for r in client.read('/index').splitlines()) |