Close inherited file descriptors in forked child processes - #886
Open
wharris623 wants to merge 1 commit into
Open
Close inherited file descriptors in forked child processes#886wharris623 wants to merge 1 commit into
wharris623 wants to merge 1 commit into
Conversation
ScriptController::StartProcess() forks a child to run unrar/7z or an
extension/post-processing script, but only ever closes the pipe fds
it created itself before calling execvp(). Any other file descriptor
the main process happens to have open at that exact moment is
inherited unchanged into the child (execvp does not close fds unless
they are marked close-on-exec).
In practice this means: if the main process has a file briefly open
for one queue item (e.g. mid-rename during Move) at the instant it
forks a child for a completely unrelated item (another unpack, or an
extension script), that unrelated child inherits the handle. The
original file then appears "busy" to the OS/filesystem for as long as
that unrelated child keeps running, and the Move fails with EBUSY
("Resource busy" on NFS mounts, surfaced as a "silly rename" .nfsXXXX
file).
Fix: compute the process's open-fd ceiling via sysconf(_SC_OPEN_MAX)
before fork() (sysconf is not async-signal-safe, so it can't be called
in the child), then in the child, after the stdin/stdout/stderr pipe
fds are wired up, close every other fd before execvp().
dnzbk
self-requested a review
August 8, 2026 07:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When downloading multiple files at a time over a 100GbE local LAN connection to an NVMe RAID10 pool on a TrueNAS box, I noticed that sometimes — like when
unraris running for one download and one of the rename/flatten extension scripts runs for another — NZBGet would lock a previously-opened file that it isn't even touching.I was initially thinking this was a TrueNAS or Ubuntu issue, but with the assistance of Claude Code, we wrote a small monitoring tool to catch it happening live, and the logs showed it's actually a behavior in how NZBGet handles its extension scripts: when NZBGet launches
unraror any of its other scripts to work on a second download, it locks the previously-opened file from a completely unrelated download even though the new process never touches it. On NFS this shows up as.nfs0000...silly-rename files, and it's a real problem downstream — Sonarr/Radarr don't see the actual file until the hung process finally times out.Root cause:
ScriptController::StartProcess()forks a child to rununrar/7zor an extension/post-processing script, but only closes the pipe fds it created itself before callingexecvp(). Any other file descriptor the main process happens to have open at that exact moment (e.g. mid-rename on an unrelated queue item) is inherited unchanged into the child, sinceexecvp()does not close fds unless they're marked close-on-exec.Fix: compute the process's open-fd ceiling via
sysconf(_SC_OPEN_MAX)beforefork()(sincesysconfis not async-signal-safe and can't be called in the child), then in the child, after stdin/stdout/stderr are wired up to the pipes, close every other fd beforeexecvp().AI assistance
Claude did the investigation (building the monitoring tooling, tracing the root cause to the exact code) and wrote the patch. I verified the diagnosis and the fix, and worked through the testing with it.
Lib changes
N/A — no vendored libraries changed.
Testing
With Claude Code's help, we built both the original code and the patched code on a Linux machine and ran NZBGet's automated test suite (10 test groups) against both — no regressions. We also wrote a short standalone program that copies the exact clone-and-take-over process from the real code, and ran it both unpatched and patched: the patched version closed the file handles correctly and didn't hold files open longer than necessary, while the unpatched version reproduced the leak.