Terminate Linux proxy watcher when parent exits - #20
Conversation
There was a problem hiding this comment.
Pull request overview
Adds OS-level cleanup for Linux proxy watcher processes when their parent exits unexpectedly.
Changes:
- Configures watcher commands with
PR_SET_PDEATHSIG. - Handles the parent-exit race after
fork. - Adds Linux-specific
libcdependency.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/platform/linux.rs |
Configures watcher child-process termination. |
Cargo.toml |
Adds Linux libc dependency. |
Cargo.lock |
Records the dependency update. |
Suppressed comments (1)
src/platform/linux.rs:190
- The new OS-level lifetime guarantee has no committed regression coverage, although this platform module already has unit and OS tests. Add a Linux subprocess test that verifies the watcher exits when its parent terminates without destructors, plus a case where a resolver created on a short-lived thread remains watched after that thread exits; the latter would also catch the parent-thread semantics of
PR_SET_PDEATHSIG.
// The parent may have exited between fork and PR_SET_PDEATHSIG.
if libc::getppid() != expected_parent {
libc::raise(libc::SIGTERM);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| [target.'cfg(target_os = "linux")'.dependencies] | ||
| libc = "0.2" |
| command.pre_exec(move || { | ||
| if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) == -1 { |
|
@microsoft-github-policy-service agree |
|
I am replying here because the repository disables GitHub Issues. IOW, I am not reviewing the PR. The issue not only leaks inotify instances, but also leaks ~4 GiB (!) shmem after closing VS Code. In an environment with a 16 GiB The issue, ostensibly, consists of two dedicated bugs:
In actual fact, the methodology of using dconf or gesttings utilities to monitor system proxy change events is dead wrong from the very beginning. Calling a long-running external utility from Electron's root process is the problem itself. The codebase smells like vibe-coded. If this is the case, please tell the coding agent to check how Chromium implements the same functionality correctly using gio: https://github.com/chromium/chromium/blob/e5ad1c8587d09f5dbafcaf873c77982ebd13573b/net/proxy_resolution/proxy_config_service_linux.cc#L314. VS Code is already dynamically linked against |
|
Continuing in #21, thanks! |
Summary
Terminate the Linux
dconf watch/gsettings monitorchild automatically when the process that created it exits, including shutdown paths where Rust destructors are not run.Problem
While repeatedly launching and closing a VS Code Extension Development Host on Linux, I observed one additional process remaining after every closed window:
The orphaned processes were reparented to the user systemd process and did not disappear over time. Each process owns an inotify instance. After enough development-host restarts, they exhausted the user's
fs.inotify.max_user_instanceslimit (128 in the reproduced environment). New filesystem watchers then failed withEMFILE/Too many open files, causing VS Code extension filesystem change notifications to stop.Closing the VS Code window normally, rather than killing it, produced the same leak.
The existing
Watcher::dropimplementation correctly kills and waits for the child. However, process shutdown does not guarantee that Rust destructors run. A process-owned watcher therefore needs an OS-level lifetime relationship as a fallback.The repository currently has GitHub Issues disabled (
has_issues: false), despiteSUPPORT.mdlinking to Issues, so I am submitting the small fix directly as a pull request and documenting the complete report here.Fix
Before executing either Linux watcher command, set
PR_SET_PDEATHSIGtoSIGTERM. The kernel will then terminate the watcher when its creating process dies.The code also compares the post-
prctlparent PID with the PID captured beforefork, covering the race where the parent exits before the child installs its parent-death signal.The existing
Dropcleanup remains unchanged and continues to handle normal resolver disposal.This is deliberately limited to the Linux child-process setup. It does not change the public API, resolver ownership, Node bindings, or behavior on other platforms.
Verification
process::exitwithout running destructors.Result: 59 passed, 0 failed.