Skip to content

Commit 170e07c

Browse files
committed
Close inherited file descriptors in forked child processes
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().
1 parent 6719a67 commit 170e07c

1 file changed

Lines changed: 21 additions & 0 deletions

File tree

daemon/util/ScriptController.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,15 @@ void ScriptController::StartProcess(int* pipein, int* pipeout)
579579
}
580580
#endif
581581

582+
// Determine the highest fd we might need to close in the child, before
583+
// forking - sysconf() is not guaranteed async-signal-safe, so it must
584+
// not be called after fork() in the child branch below.
585+
long openMax = sysconf(_SC_OPEN_MAX);
586+
if (openMax < 0)
587+
{
588+
openMax = 1024;
589+
}
590+
582591
debug("forking");
583592
pid_t pid = fork();
584593

@@ -619,6 +628,18 @@ void ScriptController::StartProcess(int* pipein, int* pipeout)
619628
close(pout[1]);
620629
}
621630

631+
// Close all other inherited file descriptors. Without this, any
632+
// handle the parent process happens to have open at the moment of
633+
// fork() - e.g. a file mid-rename during download post-processing -
634+
// leaks into this child and keeps that unrelated file "busy" for as
635+
// long as the child runs, causing spurious EBUSY / NFS "Resource
636+
// busy" failures on the parent's operation. close() is
637+
// async-signal-safe so this is safe to do here.
638+
for (int fd = 3; fd < (int)openMax; fd++)
639+
{
640+
close(fd);
641+
}
642+
622643
#ifdef CHILD_WATCHDOG
623644
write(1, "\n", 1);
624645
fsync(1);

0 commit comments

Comments
 (0)