diff options
Diffstat (limited to 'src/detached')
| -rw-r--r-- | src/detached/server.zig | 73 |
1 files changed, 56 insertions, 17 deletions
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; + }, }; } |