diff options
| -rw-r--r-- | docs/design.typ | 5 | ||||
| -rw-r--r-- | docs/detached.md | 15 | ||||
| -rw-r--r-- | docs/lsp.md | 14 | ||||
| -rw-r--r-- | src/detached/server.zig | 73 | ||||
| -rw-r--r-- | src/fs_service.zig | 11 | ||||
| -rw-r--r-- | src/fuse.zig | 7 |
6 files changed, 84 insertions, 41 deletions
diff --git a/docs/design.typ b/docs/design.typ index d322920a..ccb94a08 100644 --- a/docs/design.typ +++ b/docs/design.typ @@ -1157,8 +1157,9 @@ running in pane 3 is still running and has been scrolling into the core the whol time. Of the twenty-one `VTable` methods, the detached core's `Session` implements -sixteen and leaves five null: `push_post_present`, `pull_gpio_toggle`, -`pull_lsp`, `pull_pipe`, `push_fs_reply`. +seventeen and leaves four null: `push_post_present`, `pull_gpio_toggle`, +`pull_lsp`, `pull_pipe`. It mounts its own `/dev/fuse` and polls it in the same +`poll(2)` as its frontends, so `--fs` works in a daemon and needs no thread. Nothing blocks indefinitely, and that property is what a detached session is *for*. Every descriptor is non-blocking; the single `poll(2)` is the only place diff --git a/docs/detached.md b/docs/detached.md index b4dde59c..d849c810 100644 --- a/docs/detached.md +++ b/docs/detached.md @@ -255,14 +255,13 @@ Stated rather than papered over: * **The screen is shared, at the smallest common grid.** Two frontends of different sizes converge on the smaller; the larger window letterboxes. Same semantics as tmux. -* **No LSP, no selection pipe, no `--fs` control filesystem** in a detached - session. The daemon implements **sixteen** of `Host.VTable`'s **twenty-one** - methods — FEWER than the tty and SDL shells, which install nineteen each, - everything but `pull_gpio_toggle` and `push_detach` — and it is the only host - that implements `push_detach` at all. The five it leaves null divide cleanly. - Three are real losses: `pull_lsp` and `pull_pipe` want a - worker pool this deliberately single-threaded loop has not got, and - `push_fs_reply` wants a `/dev/fuse` this process never mounted. They fall back +* **No LSP and no selection pipe** in a detached session. The daemon implements + **seventeen** of `Host.VTable`'s **twenty-one** methods — fewer than the tty + and SDL shells, which install nineteen each, everything but + `pull_gpio_toggle` and `push_detach` — and it is the only host that + implements `push_detach` at all. The four it leaves null divide cleanly. + Two are real losses: `pull_lsp` and `pull_pipe` want a worker pool this + deliberately single-threaded loop has not got. They fall back to the core's in-process defaults rather than failing, so the features are quiet rather than broken. The other two are not losses at all: there is no moment "after the frame is on screen" for a process with no screen diff --git a/docs/lsp.md b/docs/lsp.md index 1adb397b..b35c8eeb 100644 --- a/docs/lsp.md +++ b/docs/lsp.md @@ -39,8 +39,8 @@ shell and the ESP32-P4 object: there `lsp.supports` is empty, which makes response; a host that links a backend would add both. **In a DETACHED session every query answers EMPTY.** -`src/detached/server.zig`'s vtable implements sixteen of `host.zig`'s -twenty-one methods, and `pull_lsp` is one of the five it leaves null — a +`src/detached/server.zig`'s vtable implements seventeen of `host.zig`'s +twenty-one methods, and `pull_lsp` is one of the four it leaves null — a worker pool is precisely what its deliberately single-threaded loop does not have. A null method is NOT automatically a dropped effect: `perform` decides that per arm, and the `.lsp` arm's answer is to synthesise one on the spot — @@ -69,16 +69,14 @@ has moved by the time the prong runs, the guard fails, and the Tab really is eaten. The local shells have the same race over a wider window, so this is a property of the late-indent repair rather than of detaching. -None of the detached core's five null methods silently drops a reachable +None of the detached core's four null methods silently drops a reachable effect. `pull_lsp` and `pull_pipe` have the fallbacks above; `pull_gpio_toggle` is `orelse return Error.NoPads` (`board_memory.zig`), which lands on the message row; `push_post_present` is a `pump` hook fired after presenting, and there is -nothing to notify in a process with no screen; and `push_fs_reply` is the one -`perform` arm with no fallback at all, but it is only ever emitted in answer to -an `Event.fs_req`, which is not on the wire (`wire.zig`: it has no `ClientTag`, -because neither half of that pair may cross an attachment) and which the -daemon raises none of, mounting no `/dev/fuse` by its own vtable comment. +nothing to notify in a process with no screen. `push_fs_reply` was listed here +too until the daemon began mounting its own `/dev/fuse`; it implements that one +now, and `--fs` works in a detached session. Four rules make it safe: diff --git a/src/detached/server.zig b/src/detached/server.zig index d8373149..8e6b92d4 100644 --- a/src/detached/server.zig +++ b/src/detached/server.zig @@ -394,11 +394,22 @@ const Source = union(enum) { client: u8, pty: u8, inotify, - /// The acme filesystem's descriptor. Its arm does nothing: being in the set - /// is the whole point, because a readable `/dev/fuse` must end the sleep so - /// that `pollFrame` — which runs after `pull_wait_input` returns — reaches - /// the drain. Answering it here instead would re-enter the core from inside - /// its own `pump`. + /// The acme filesystem's descriptor. Its arm does one thing only: notice + /// that the connection has gone. Being in the set is otherwise the whole + /// point, because a readable `/dev/fuse` must END THE SLEEP so that + /// `pollFrame` — which runs after `pull_wait_input` returns — reaches the + /// drain. + /// + /// The drain is not done HERE, and the reason is not re-entrancy: both + /// `pull_wait_input` and `push_poll_frame` are called from inside + /// `Pardes.pump`, so either would re-enter. It is that `dispatch` is + /// mid-iteration over the `fds[0..n]`/`src[0..n]` SNAPSHOT `waitInput` + /// built, and a filesystem request reaches `core.perform`, which drains the + /// whole effect ring — including a `push_spawn`, whose `Session.spawn` + /// closes a pane's master and forks a new one. A later `.pty` entry in the + /// same pass would then apply the old descriptor's `revents` to a brand new + /// one, and the `fd < 0` guard cannot see it because the fd is valid, + /// merely different. Same hazard as the client slots, same reason. fuse, }; @@ -486,9 +497,18 @@ pub const Session = struct { pub fn deinit(s: *Session) void { // First, and before the pane shells: a script blocked on `event` is // holding a kernel request, and `Fs.deinit` answers everything still - // parked and aborts the connection before unmounting. Leaving it until - // after the harvest would leave that reader in uninterruptible sleep - // for as long as the harvest takes. + // parked and ABORTS THE CONNECTION before unmounting, so that reader + // wakes with ENODEV while its own shell is still alive to run its exit + // path. After `closePty` it would be woken by a hangup instead, with + // nothing left to exit into. + // + // Not, as this comment first claimed, because the harvest takes time: + // `harvest` is `waitpid(WNOHANG)` in a loop and blocks for nothing. The + // one step here that CAN take milliseconds is this one, because + // `Fs.deinit` forks `fusermount3` and waits for it untimed — which is + // also why the `.quit` owed to every frontend now queues behind a + // subprocess. Worth knowing; not worth reordering, because the shells + // matter more than the milliseconds. if (s.fs) |f| { f.deinit(); s.fs = null; @@ -1083,11 +1103,16 @@ pub const Session = struct { /// Collect every child that has exited. /// /// `waitpid(-1)` and not a pid list, because the only children this process - /// forks are pane shells (`host_io.forkShell`) — so "any exited child" and - /// "an exited pane shell" are the same set — and because the pids a list - /// would hold are exactly the ones it cannot help with: a respawn closes a - /// master, and the shell that gets the hangup exits some milliseconds later - /// with its slot already reused by a different shell. + /// LEAVES UNREAPED are pane shells (`host_io.forkShell`) — so "any exited + /// child" and "an exited pane shell" are the same set — and because the pids + /// a list would hold are exactly the ones it cannot help with: a respawn + /// closes a master, and the shell that gets the hangup exits some + /// milliseconds later with its slot already reused by a different shell. + /// Anything else this process forks — `fusermount3`, from `Fs.mount`, + /// `sweepStale` and `Fs.deinit` — is reaped by its own spawner with a + /// pid-specific blocking wait before control returns here, so the set this + /// sees is still only shells. A future worker that forks and does not wait + /// would break that, and this is the sentence it has to come back and edit. /// /// Nothing here waits, so a session whose shells are all running pays one /// syscall that returns 0. Called once per poll round and again wherever a @@ -1305,7 +1330,17 @@ pub const Session = struct { // `pollFrame` after `pull_wait_input` returns reaches the drain. The // desktop shells buy the same wake with a thread; one poll slot is // cheaper and cannot race the loop. - if (s.fs) |f| if (f.fd >= 0) { + // ...and NOT once it is dead. `fuse_dev_poll` answers `EPOLLERR` as soon + // as the connection is gone, POSIX reports `POLLERR` whatever the events + // mask asked for, and this arm cannot consume it — so an external + // `fusermount3 -u`, a sysfs abort, or systemd taking `/run/user/$UID` + // away at final logout (exactly when a detached session is supposed to + // keep running) would make `poll(2)` return instantly, forever, and burn + // a whole core for the life of the daemon. Measured at 100% of one CPU + // before this guard. fuse.zig's own poll thread has carried the + // equivalent check all along, which is why the desktop shells never + // showed it and this loop did. + if (s.fs) |f| if (f.fd >= 0 and !f.dead) { fds[n] = .{ .fd = f.fd, .events = poll_in, .revents = 0 }; src[n] = .fuse; n += 1; @@ -1413,9 +1448,13 @@ pub const Session = struct { } }, .inotify => if (pfd.revents & poll_in != 0) s.drainInotify(), - // Nothing. See `Source.fuse`: the wake IS the work, and the drain - // belongs to `pollFrame`, where re-entering the core is legal. - .fuse => {}, + // The only revents worth a word: a dead connection must leave the + // set, or the `POLLERR` it reports on every future poll spins the + // loop. `Fs.next` would set `dead` on its first failed read anyway; + // saying it here costs nothing and saves the one spinning round. + .fuse => if (pfd.revents & (poll_hup | poll_err | poll_nval) != 0) { + if (s.fs) |f| f.dead = true; + }, }; } diff --git a/src/fs_service.zig b/src/fs_service.zig index 7821e850..b440c2c2 100644 --- a/src/fs_service.zig +++ b/src/fs_service.zig @@ -43,10 +43,13 @@ const max_batch = 64; /// listener beside the mount a matter of writing one more implementation /// rather than of teaching this file about it. /// -/// It is deliberately NOT a Zig interface with `anytype`: the two callers -/// (tty.zig and gui.zig) store the transport in a struct field across frames, -/// so it has to be a value with a runtime type, which is a vtable — the same -/// shape and the same reasoning as `host.VTable`. +/// It is deliberately NOT a Zig interface with `anytype`: `drain` is ONE +/// function reached from four call sites (tty.zig, gui.zig twice, and the +/// daemon), and the second transport this seam exists for is chosen at RUN +/// time, so it has to be a value with a runtime type — which is a vtable, the +/// same shape and the same reasoning as `host.VTable`. Nothing holds a +/// `Transport` across a frame: every caller builds one inline from whatever it +/// has, which is why the thunks matter and the struct does not. /// /// The ORDER contract stays where it was, in `drain`, because it belongs to /// the caller rather than to any implementor: `retry()` to null first, then diff --git a/src/fuse.zig b/src/fuse.zig index 3a903e8b..3bd263bd 100644 --- a/src/fuse.zig +++ b/src/fuse.zig @@ -59,8 +59,11 @@ const libc = std.c; const linux = std.os.linux; const acmefs = @import("acmefs.zig"); /// Only for `Transport`, the three-function shape this mount presents to the -/// host loop. No cycle: `fs_service` names no type from here any more, which -/// is the point of the seam. +/// host loop. This IS a cycle — `fs_service` imports this file back for +/// `Fs.mount`, `sweepStale` and `exportPaneEnv`, and still names `*Fs` in three +/// of its own signatures — and Zig accepts it because imports are analysed +/// lazily. What the seam removed is `drain`'s dependency on the concrete type, +/// not the file's dependency on this one. Do not read it as more than that. const fs_service = @import("fs_service.zig"); /// Everything below the mount is Linux kernel ABI. Off Linux the module still |