Skip to content

Commit

Permalink
Fix Bugzilla 24715 - std/process: Default to libc closefrom in spaw…
Browse files Browse the repository at this point in the history 
…nProcessPosix

The current implementation of spawnProcessPosix is broken on systems
with a large `ulimit -n` because it always OOMs making it impossible
to spawn processes. Using the libc implementation, when available, for
doing file descriptor operations en-mass partially solves this problem.

Signed-off-by: Andrei Horodniceanu <[email protected]>
  • Loading branch information
@the-horo
the-horo committed Aug 26, 2024
1 parent f7e523b commit 48d581a
Showing 1 changed file with 102 additions and 47 deletions.
149 changes: 102 additions & 47 deletions std/process.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,7 @@ version (Posix) private enum InternalError : ubyte
doubleFork,
malloc,
preExec,
closefds_dup2,
}

/*
Expand Down Expand Up @@ -1008,7 +1009,7 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
if (config.flags & Config.Flags.detached)
close(pidPipe[0]);
close(forkPipe[0]);
immutable forkPipeOut = forkPipe[1];
auto forkPipeOut = forkPipe[1];
immutable pidPipeOut = pidPipe[1];

// Set the working directory.
Expand Down Expand Up @@ -1042,56 +1043,106 @@ private Pid spawnProcessPosix(scope const(char[])[] args,

if (!(config.flags & Config.Flags.inheritFDs))
{
// NOTE: malloc() and getrlimit() are not on the POSIX async
// signal safe functions list, but practically this should
// not be a problem. Java VM and CPython also use malloc()
// in its own implementation via opendir().
import core.stdc.stdlib : malloc;
import core.sys.posix.poll : pollfd, poll, POLLNVAL;
import core.sys.posix.sys.resource : rlimit, getrlimit, RLIMIT_NOFILE;

// Get the maximum number of file descriptors that could be open.
rlimit r;
if (getrlimit(RLIMIT_NOFILE, &r) != 0)
{
abortOnError(forkPipeOut, InternalError.getrlimit, .errno);
}
immutable maxDescriptors = cast(int) r.rlim_cur;

// The above, less stdin, stdout, and stderr
immutable maxToClose = maxDescriptors - 3;
version (FreeBSD)
import core.sys.freebsd.unistd : closefrom;
else version (OpenBSD)
import core.sys.openbsd.unistd : closefrom;

// Call poll() to see which ones are actually open:
auto pfds = cast(pollfd*) malloc(pollfd.sizeof * maxToClose);
if (pfds is null)
{
abortOnError(forkPipeOut, InternalError.malloc, .errno);
}
foreach (i; 0 .. maxToClose)
static if (!__traits(compiles, closefrom))
{
pfds[i].fd = i + 3;
pfds[i].events = 0;
pfds[i].revents = 0;
}
if (poll(pfds, maxToClose, 0) >= 0)
{
foreach (i; 0 .. maxToClose)
{
// don't close pipe write end
if (pfds[i].fd == forkPipeOut) continue;
// POLLNVAL will be set if the file descriptor is invalid.
if (!(pfds[i].revents & POLLNVAL)) close(pfds[i].fd);
}
}
else
{
// Fall back to closing everything.
foreach (i; 3 .. maxDescriptors)
{
if (i == forkPipeOut) continue;
close(i);
// FIXME: This implementation crashes the system when RLIMIT_NOFILE
// has a big value. For a possible solution see:
// https://github.com/dlang/phobos/pull/8990
void fallback (int lowfd) {
// NOTE: malloc() and getrlimit() are not on the POSIX async
// signal safe functions list, but practically this should
// not be a problem. Java VM and CPython also use malloc()
// in its own implementation via opendir().
import core.stdc.stdlib : malloc;
import core.sys.posix.poll : pollfd, poll, POLLNVAL;
import core.sys.posix.sys.resource : rlimit, getrlimit, RLIMIT_NOFILE;

// Get the maximum number of file descriptors that could be open.
rlimit r;
if (getrlimit(RLIMIT_NOFILE, &r) != 0)
{
abortOnError(forkPipeOut, InternalError.getrlimit, .errno);
}
immutable maxDescriptors = cast(int) r.rlim_cur;

immutable maxToClose = maxDescriptors - lowfd;

// Call poll() to see which ones are actually open:
auto pfds = cast(pollfd*) malloc(pollfd.sizeof * maxToClose);
if (pfds is null)
{
abortOnError(forkPipeOut, InternalError.malloc, .errno);
}
foreach (i; 0 .. maxToClose)
{
pfds[i].fd = i + lowfd;
pfds[i].events = 0;
pfds[i].revents = 0;
}
if (poll(pfds, maxToClose, 0) >= 0)
{
foreach (i; 0 .. maxToClose)
{
// POLLNVAL will be set if the file descriptor is invalid.
if (!(pfds[i].revents & POLLNVAL)) close(pfds[i].fd);
}
}
else
{
// Fall back to closing everything.
foreach (i; lowfd .. maxDescriptors)
{
close(i);
}
}
}

// closefrom may not be available on the version of glibc we build against.
// Until we find a way to perform this check we will try to use dlsym to
// check for the function. See: https://github.com/dlang/phobos/pull/9048
version (CRuntime_Glibc)
void closefrom (int lowfd) {
static bool tryGlibcClosefrom (int lowfd) {
import core.sys.posix.dlfcn : dlopen, dlclose, dlsym, dlerror, RTLD_LAZY;

void *handle = dlopen("libc.so.6", RTLD_LAZY);
if (!handle)
return false;
scope(exit) dlclose(handle);

// Clear errors
dlerror();
alias closefromT = extern(C) void function(int) @nogc @system nothrow;
auto closefrom = cast(closefromT) dlsym(handle, "closefrom");
if (dlerror())
return false;

closefrom(lowfd);
return true;
}

if (!tryGlibcClosefrom(lowfd))
fallback(lowfd);
}
else
alias closefrom = fallback;
}

// We need to close all open file descriptors excluding std{in,out,err}
// and forkPipeOut because we still need it.
// Since the various libc's provide us with `closefrom` move forkPipeOut
// to position 3, right after STDERR_FILENO, and close all FDs following that.
if (dup2(forkPipeOut, 3) == -1)
abortOnError(forkPipeOut, InternalError.closefds_dup2, .errno);
forkPipeOut = 3;
// forkPipeOut needs to be closed after we call `exec`.
setCLOEXEC(forkPipeOut, true);
closefrom(forkPipeOut + 1);
}
else // This is already done if we don't inherit descriptors.
{
Expand Down Expand Up @@ -1205,6 +1256,10 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
case InternalError.preExec:
errorMsg = "Failed to execute preExecFunction or preExecDelegate";
break;
case InternalError.closefds_dup2:
assert(!(config.flags & Config.Flags.inheritFDs));
errorMsg = "Failed to close inherited file descriptors";
break;
case InternalError.noerror:
assert(false);
}
Expand Down

0 comments on commit 48d581a

Please sign in to comment.