fix(state,desktop,runtime): EMFILE cascade — bounded WAL read pool, orphan serve reap, nofile floor - #83406
Conversation
…ssionDB x thread)
…urns
Review of this PR was right that maxsize=8 bounds the wrong thing. The
LifoQueue caps how many connections are RETURNED; _checkout_read_conn opened
unconditionally on a miss, so N readers arriving on a cold pool all missed, all
opened, and peaked at N. The surplus was closed on release, so nothing
accumulated forever -- but EMFILE is a peak-instant condition and the burst
that empties the pool is exactly the burst that exhausts the fd table, so the
original wedge was still reachable. Measured on the previous commit: 64
concurrent readers held 64 live connections at once.
A connection now holds a permit for its whole lifetime -- acquired in
_get_read_conn() before the open, released in _close_read_conn() after the
close -- so open+checked-out is bounded together. A pool hit costs no permit
because the connection it hands back already holds one, which leaves
_get_read_conn() as the only place that can open. The acquire is non-blocking:
past the ceiling readers fall back to the locked writer connection rather than
queueing, since blocking would convert descriptor exhaustion into a stall,
which is the same outage with a different stack trace. Same burst now peaks at
8. BoundedSemaphore rather than Semaphore so an unpaired release raises instead
of silently widening the ceiling.
Two latent leaks in the same function, found while doing this:
- a CJK extension load that failed after a successful open returned None
without closing the connection, leaking a descriptor the tracking registry
still counted -- the same leak shape one level down;
- any non-sqlite3.Error between open and return stranded a permit
permanently, which would ratchet the ceiling down to zero and silently
demote every later read to the writer lock.
On the test: the existing one joins every worker before counting, so it
measures the pool at rest and structurally cannot observe peak -- which is why
this got through. The new one uses a barrier so all 64 workers hold their
connections until every worker has checked out, making the count taken at that
moment the actual simultaneous peak. Verified it fails against the previous
commit (64 checked out, 65 live) and passes at 8/9. Also covers the
writer-connection fallback, permit recovery after a failed open, and that
close() releases exactly the permits it drained.
…+ group-kill An unclean desktop exit (crash / SIGKILL / update handoff) stranded every `hermes serve` profile backend as an orphan (ppid=1) still serving, each holding its MCP child subtree — 31 orphans / ~1.3 GiB RSS on one install. Root causes + fixes: - serve had no parent-death watchdog: add _start_parent_death_watchdog() in web_server.py (mirrors slash_worker.py), gated on HERMES_PARENT_PID; os._exit cascades to MCP watchdogs. No-op for standalone `hermes serve`. - desktop passes HERMES_PARENT_PID in both serve spawn env blocks (main.ts). - POSIX teardown now group-kills (process.kill(-pid, ...)) so MCP grandchildren die too (backend-child.ts + waitForBackendExit SIGKILL fallback). Windows path unchanged (forceKillProcessTree). Tests updated + passing.
When Desktop exits uncleanly, leftover `hermes serve --host 127.0.0.1 --port 0` processes can be reparented to pid 1 and keep full MCP trees alive. The next boot then stacks another backend on top of the corpses until EMFILE kills sidebar/session APIs and tabs disappear. - Detect Desktop-local serve shape (loopback + ephemeral port 0) - Only reap processes whose ppid is 0/1 (true orphans) - Spare fixed-port remote serves (e.g. --port 9119) and HERMES_DESKTOP_CHILD_PID - Run at Desktop backend start (HERMES_DESKTOP=1) before parent-death watchdog Complements parent-death watchdog (prevents future orphans) and configurable nofile soft limit (capacity floor). Together these stop the multi-backend pile-up cascade observed on macOS Desktop SSH/local installs.
…plist launchd starts children with soft nofile=256; hermes gateway start rewrites the plist and previously stripped any manually-added SoftResourceLimits, silently reintroducing EMFILE crashes under load. The plist generator now embeds the configured runtime.nofile_soft_limit so the persisted service definition and the in-process floor share one knob.
Production incident: the orphan reap killed a legitimate SSH remote backend started by another client machine. Its process sat at ppid 1 with the same cmdline shape as a genuine orphan, and the exclusion list only covered THIS app instance's children — ownership by OTHER clients was invisible. The reap now treats every backend.lock.json under ~/.hermes/desktop-ssh/*/ as an ownership claim: lock payloads are schema-validated (mirroring remote-lifecycle.ts) and their PIDs are excluded both before the scan and re-checked after it (defense in depth against a lock written mid-scan). Regression tests cover the exact incident shape: a lock-owned PID and a genuine orphan with identical process shapes — only the orphan is reaped. Also: fold the new single-field `runtime` config category into `agent` (_CATEGORY_MERGE) and fix an env leak in the serve-startup test (HERMES_SERVE_HEADLESS restored via monkeypatch) so the combined suites run green in any order.
On Desktop serve startup, reap orphan gateway processes (PPID=1) left behind by a previous serve session that exited abnormally. This prevents the old and new gateways from racing for the same QQ WebSocket credential, which splits messages across parallel session trees (#77276).
૮ >ﻌ< ა ci reviewran on 6558cd7 — fix: use psutil.pid_exists for orphan-reap liveness probe (W
|
…otgun lint) os.kill(pid, 0) sends CTRL_C_EVENT on Windows (bpo-14484). The reap path is POSIX-only, but the blocking lint rejects the pattern repo-wide and psutil is a core dependency.
|
The SessionDB pool and the in-process nofile floor look solid. The lifetime permit fixes the important peak-FD hole, the writer fallback is a sensible degradation path, and the close/open failure coverage is much stronger than the old per-thread connection model. I do not think the process-lifecycle portion is safe to merge yet, though. I found four blockers:
That list includes the valid PID-file owner and service-manager PIDs. On macOS, The new test only verifies that the reaper is invoked and replaces the reaper with a stub, so it cannot catch this. This path needs to preserve the valid runtime/PID-file owner and every service-managed PID, then act only on a process with positive evidence that it is an abandoned duplicate from a dead Desktop backend.
With the current spawn options, the negative-PID send normally fails with ESRCH and falls back to The unit test injects a fake
The startup reaper treats The SSH lock exclusion fixes one known false positive, but other legitimate owners remain unprotected. This should require a positive Desktop ownership record, ideally PID plus process creation time or nonce. The SIGKILL escalation should also revalidate that process identity rather than checking only
The in-process helper correctly never lowers and clamps against the hard limit. The generated launchd plist writes the configured value directly as That can lower an inherited soft limit above 4096, and it can request an invalid value when the configured target exceeds the launchd domain's hard limit. The plist path needs to resolve inherited launchd maxfiles, preserve the higher soft value, clamp to a finite hard value, and omit the absolute override if inheritance cannot be determined. The in-process helper can remain the fallback. I would split this PR and merge the bounded SessionDB pool plus the in-process nofile helper after review. The gateway reap, serve reap, POSIX group teardown, and launchd plist override should stay out until their ownership and process-group contracts are fixed. Smaller cleanup:
|
alt-glitch
added
type/bug
6 participants
Summary
Desktop becomes usable again with multiple long-running chats: the SessionDB WAL-reader fd leak is fixed with a bounded connection pool, orphaned
hermes servebackends are reaped at Desktop boot (with a parent-death watchdog preventing new ones), and the process nofile soft limit gets a configurable floor so a single busy backend can no longer wedge itself at macOS's 256-fd default. Fixes #78872, #75269; closes the EMFILE cascade behind #73066 / #77573 / #78312.Root cause chain: every anyio worker thread that ever served a read pinned one read-only SQLite connection (two fds: db + wal) for the life of the process → a single healthy
servehit RLIMIT_NOFILE 256 within ~12–20h →socket.accept()wedged while the port still probed open → Desktop "could not reach the gateway". Separately, Electron dying uncleanly strandedserve --host 127.0.0.1 --port 0children at ppid 1, stacking corpses (17 reported) each holding full MCP trees.Changes
hermes_state.py: bounded LIFO read-connection pool (max 8) with a lifetime permit per open — bounds PEAK descriptors, not just idle; degradation past the ceiling is the locked writer path, never EMFILE;close()drains from any thread (salvaged from state: pool SessionDB read connections instead of leaking one per (SessionDB x thread) #76700 by @Yishova)hermes_cli/resource_limits.py(new):runtime.nofile_soft_limitconfig (default 4096), applied at serve/gateway start; never lowers, never touches RLIM_INFINITY (salvaged from fix(runtime): make nofile soft limit configurable #77587 by @100yenadmin)hermes_cli/dashboard_procs.py: startup reap of already-orphaned Desktop-local serves — matches ONLY the--host 127.0.0.1 --port 0Desktop spawn shape at ppid 1; never touches PIDs claimed by a validbackend.lock.json(SSH remote backends legitimately sit at ppid 1) (salvaged from fix(runtime,desktop,gateway): EMFILE hardening — nofile floor, orphan serve reap, launchd plist limit persistence #78873 by @leonphull)apps/desktop/electron: parent-death watchdog + group-kill so future backend children die with Electron (salvaged from fix(desktop): reap orphaned serve backends via parent-death watchdog + group-kill #73066 by @XiaoZAZA)hermes_cli/gateway.py: launchd plist persists the nofile limit so launchd-managed serves get it too (@leonphull)Validation
close()/api/sessionsrequests against liveserveTargeted tests: 457 passed, 0 failed (
test_session_db_read_conn_pool,test_session_db_read_path_split,test_resource_limits,test_orphan_desktop_serve_reap,test_gateway_service,test_gateway_restart_loop,test_hermes_state). Desktop vitest: 14/14 (windows-child-options). Live cross-process E2E ran on Linux against real processes and a livehermes serveon port 9272 (WAL-capable SQLite 3.53.1).Contributor commits cherry-picked with authorship preserved: @Yishova (#76700), @100yenadmin (#77587), @XiaoZAZA (#73066), @leonphull (#78873), @cadezhou (#78312).
Not included (deliberately): the renderer memory fix (#71269 is draft and conflicts with the newer parts-budget live-tail from #66470-family — needs its own design pass), FTS multimodal-sentinel bloat (#69798 conflicts extensively with current hermes_state.py — separate salvage), and any state.db retention/eviction policy (#54189 is needs-decision).
Infographic