Add crash-orphan sweep for stale tap-relay processes
Direct tap+relay analog of the existing -p port-forward sweep: unlike a veth pair or a session's own bridge/persistent-namespace state, a relay process is host-global state with no automatic teardown if slocker-lite crashes before reaching its own stop_tap_relay() calls (bwrap still dies immediately via --die-with-parent, so only the relay can actually leak). tap_relay_state_path() uses the same xdg_state_dir()/ sanitize_for_filename() naming scheme as session_pid_file_path()/ port_forward_state_path(), so clean_stale_tap_relays() can cross-reference filenames against list_sessions() the same way clean_stale_port_forwards() already does. record_tap_relays()/ remove_tap_relay_record() are wired into run_container() the same two-places-split as their port-forward counterparts. Wired into clean_processes_command() (--clean-processes) alongside the two existing sweeps. Verified via a controlled scratch test, the same shape the original port-forward sweep used: a plain rootless daemonized session (no network join needed for the sweep logic itself) gave a real, live pid, alongside a hand-written matching record and a fabricated stale one in the same rootless state dir -- the matching record was left untouched, the fabricated one was correctly identified as stale and removed (kill() on its nonexistent pid failing harmlessly with ESRCH), and killing the real session then correctly swept its own now-stale record on a second run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
This commit is contained in:
@@ -124,8 +124,11 @@ Source layout (all under `src/`):
|
||||
<pid>)` line per file actually removed, then also calls
|
||||
`clean_stale_port_forwards()` (`port_forward.h`, see below — commit 6 of
|
||||
`docs/networking-design.md`'s sequence) and prints one `removed stale
|
||||
port-forward rules for '<name>-<pid>'` line per record actually swept —
|
||||
nothing is printed for sessions still running, and an empty result
|
||||
port-forward rules for '<name>-<pid>'` line per record actually swept,
|
||||
then `clean_stale_tap_relays()` (`network_tap_relay.h`, see below — the
|
||||
direct tap+relay analog of the port-forward sweep) and prints one
|
||||
`removed stale tap-relay processes for '<name>-<pid>'` line the same way
|
||||
— nothing is printed for sessions still running, and an empty result
|
||||
(nothing stale) is silent success, same convention as the rest of this
|
||||
file's list/delete commands. `Mode::exec`'s
|
||||
dispatch case is a one-line call to `exec_in_session(*args.exec_pid,
|
||||
@@ -278,7 +281,13 @@ Source layout (all under `src/`):
|
||||
(`network_tap_relay.h`) analog of `active_port_forwards` above, since a
|
||||
relay process is likewise independent host-global state (unlike a veth
|
||||
pair) that needs an explicit `stop_tap_relay()` call for each, made right
|
||||
alongside the `remove_port_forward()` loop after `run_bwrap()` returns.
|
||||
alongside the `remove_port_forward()` loop after `run_bwrap()` returns —
|
||||
paired the same two-places way with `record_tap_relays(container_name,
|
||||
pid, active_relays)` (called right after `record_port_forwards()`, same
|
||||
callback) and `remove_tap_relay_record(container_name, bwrap_pid)` (called
|
||||
right after the `stop_tap_relay()` loop) for `--clean-processes`'s own
|
||||
crash-orphan sweep (`network_tap_relay.h`'s `clean_stale_tap_relays()`,
|
||||
see below).
|
||||
- `self_test.{h,cpp}` — `run_self_tests()` implements `-t/--test`, this
|
||||
project's own built-in self-test mode (distinct from the Meson-driven
|
||||
fixture smoke test under `tests/`, described in "Build & test commands"
|
||||
@@ -1005,6 +1014,42 @@ Source layout (all under `src/`):
|
||||
specifically needs re-verification, ideally on the actual veth-less
|
||||
target device (a different kernel/environment where this dev sandbox's
|
||||
own unidentified cause may not even apply) before being relied on.
|
||||
|
||||
**Crash-orphan sweep**, the direct tap+relay analog of `port_forward.h`'s
|
||||
own (see its own entry below): unlike a veth pair or a session's own
|
||||
bridge/persistent-namespace state, a relay process is host-global state
|
||||
with no automatic teardown at all if `slocker-lite` itself is
|
||||
killed/crashes before reaching its own `stop_tap_relay()` calls (`bwrap`
|
||||
still dies immediately in that case via `--die-with-parent`, so only the
|
||||
*relay* — never the sandboxed container itself — can actually leak).
|
||||
`tap_relay_state_path()` resolves `xdg_state_dir() / "tap-relays" /
|
||||
"<container_name>-<pid>"` — the same naming scheme
|
||||
`session_pid_file_path()`/`port_forward_state_path()` already use, so
|
||||
`clean_stale_tap_relays()` can cross-reference filenames directly against
|
||||
`list_sessions()`'s own `SessionInfo::path`. `record_tap_relays()` writes
|
||||
one line per relay (`"<relay_pid> <host_tap_name>"`) to that path — a
|
||||
no-op if there's nothing to record. `clean_stale_tap_relays()`
|
||||
(`commands.cpp`'s `clean_processes_command()`, alongside
|
||||
`clean_stale_sessions()`/`clean_stale_port_forwards()`) scans that
|
||||
directory: a record whose filename doesn't match any currently-*running*
|
||||
session is stale — every relay pid listed is `SIGKILL`ed (best-effort; an
|
||||
already-dead pid, or one this process was never the parent of, isn't
|
||||
treated as an error, since this sweep runs from a *separate* later
|
||||
invocation that can't `waitpid()` an orphan it didn't fork — its true
|
||||
parent's own exit, or `init` after reparenting, reaps it) before the
|
||||
record file itself is deleted; a record whose session is still running is
|
||||
left completely untouched. **Verified via a controlled scratch test**,
|
||||
the same shape `port_forward.h`'s own sweep test used: root wasn't needed
|
||||
for the sweep *logic* itself (only real tap/bridge creation needs it),
|
||||
so this ran as a plain rootless daemonized session (`-D`, no `-n`) to get
|
||||
a real, live pid + container name, alongside two hand-written record
|
||||
files in that same rootless `$XDG_STATE_HOME` — one *matching* the live
|
||||
session (confirmed left untouched by `--clean-processes`) and one
|
||||
*fabricated* for a nonexistent pid (confirmed identified as stale, its
|
||||
`kill()` attempt failing harmlessly with `ESRCH`, and its record file
|
||||
actually removed, reported as `removed stale tap-relay processes for
|
||||
'faketest-999999'`); killing the real session and re-running
|
||||
`--clean-processes` then correctly swept its own now-stale record too.
|
||||
- `session_cgroup.{h,cpp}` — gives `--kill` (`kill_session.{h,cpp}`, see
|
||||
below) a reliable way to find every process a session ever started, however
|
||||
deeply forked/daemonized/reparented, by putting it in a dedicated cgroup v2
|
||||
|
||||
Reference in New Issue
Block a user