Skip to content

Commit bc0204b

Browse files
committed
Address review feedback on PR dlang#10969
- Drop explicit `long` type, use inferred type from r.rlim_cur (0xEAB) - Revert else+if merge back to original else { if ... } structure (0xEAB) - Guard test with static if (rlim_t.sizeof > int.sizeof) for platform safety, and use cast(rlim_t) instead of cast(ulong) (ibuclaw)
1 parent 8611dbb commit bc0204b

1 file changed

Lines changed: 71 additions & 64 deletions

File tree

std/process.d

Lines changed: 71 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,7 +1046,7 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
10461046
if (getrlimit(RLIMIT_NOFILE, &r) != 0)
10471047
abortOnError(forkPipeOut, InternalError.getrlimit, .errno);
10481048

1049-
immutable long maxDescriptors = r.rlim_cur;
1049+
immutable maxDescriptors = r.rlim_cur;
10501050

10511051
// Missing druntime declaration
10521052
pragma(mangle, "dirfd")
@@ -1080,52 +1080,55 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
10801080
close(fd);
10811081
}
10821082
}
1083-
else if (maxDescriptors > 0 && maxDescriptors <= 128*1024)
1083+
else
10841084
{
1085-
// This is going to allocate 8 bytes for each possible file descriptor from lowfd to rlim_cur.
1086-
// NOTE: malloc() and getrlimit() are not on the POSIX async
1087-
// signal safe functions list, but practically this should
1088-
// not be a problem. Java VM and CPython also use malloc()
1089-
// in its own implementation via opendir().
1090-
import core.stdc.stdlib : malloc;
1091-
import core.sys.posix.poll : pollfd, poll, POLLNVAL;
1092-
1093-
immutable int maxToClose = cast(int)(maxDescriptors - lowfd);
1094-
1095-
// Call poll() to see which ones are actually open:
1096-
auto pfds = cast(pollfd*) malloc(pollfd.sizeof * maxToClose);
1097-
if (pfds is null)
1098-
{
1099-
abortOnError(forkPipeOut, InternalError.malloc, .errno);
1100-
}
1101-
1102-
foreach (i; 0 .. maxToClose)
1103-
{
1104-
pfds[i].fd = i + lowfd;
1105-
pfds[i].events = 0;
1106-
pfds[i].revents = 0;
1107-
}
1108-
1109-
if (poll(pfds, maxToClose, 0) < 0)
1110-
// couldn't use poll, use the slow path.
1111-
goto LslowClose;
1112-
1113-
foreach (i; 0 .. maxToClose)
1085+
if (maxDescriptors <= 128*1024)
11141086
{
1115-
// POLLNVAL will be set if the file descriptor is invalid.
1116-
if (!(pfds[i].revents & POLLNVAL)) close(pfds[i].fd);
1087+
// This is going to allocate 8 bytes for each possible file descriptor from lowfd to rlim_cur.
1088+
// NOTE: malloc() and getrlimit() are not on the POSIX async
1089+
// signal safe functions list, but practically this should
1090+
// not be a problem. Java VM and CPython also use malloc()
1091+
// in its own implementation via opendir().
1092+
import core.stdc.stdlib : malloc;
1093+
import core.sys.posix.poll : pollfd, poll, POLLNVAL;
1094+
1095+
immutable maxToClose = cast(int)(maxDescriptors - lowfd);
1096+
1097+
// Call poll() to see which ones are actually open:
1098+
auto pfds = cast(pollfd*) malloc(pollfd.sizeof * maxToClose);
1099+
if (pfds is null)
1100+
{
1101+
abortOnError(forkPipeOut, InternalError.malloc, .errno);
1102+
}
1103+
1104+
foreach (i; 0 .. maxToClose)
1105+
{
1106+
pfds[i].fd = i + lowfd;
1107+
pfds[i].events = 0;
1108+
pfds[i].revents = 0;
1109+
}
1110+
1111+
if (poll(pfds, maxToClose, 0) < 0)
1112+
// couldn't use poll, use the slow path.
1113+
goto LslowClose;
1114+
1115+
foreach (i; 0 .. maxToClose)
1116+
{
1117+
// POLLNVAL will be set if the file descriptor is invalid.
1118+
if (!(pfds[i].revents & POLLNVAL)) close(pfds[i].fd);
1119+
}
11171120
}
1118-
}
1119-
else
1120-
{
1121-
LslowClose:
1122-
// Fall back to closing everything up to a sane limit.
1123-
// When rlim_cur is huge (e.g. unlimited), cap to avoid
1124-
// iterating over billions of file descriptors.
1125-
immutable long closeMax = maxDescriptors > 1_048_576 ? 1_048_576 : maxDescriptors;
1126-
foreach (i; lowfd .. cast(int) closeMax)
1121+
else
11271122
{
1128-
close(i);
1123+
LslowClose:
1124+
// Fall back to closing everything up to a sane limit.
1125+
// When rlim_cur is huge (e.g. unlimited), cap to avoid
1126+
// iterating over billions of file descriptors.
1127+
immutable closeMax = maxDescriptors > 1_048_576 ? 1_048_576 : maxDescriptors;
1128+
foreach (i; lowfd .. cast(int) closeMax)
1129+
{
1130+
close(i);
1131+
}
11291132
}
11301133
}
11311134
}
@@ -1801,32 +1804,36 @@ version (Posix) @system unittest
18011804
// massive malloc that would fail with "Cannot allocate memory".
18021805
version (Posix) @system unittest
18031806
{
1804-
import core.sys.posix.sys.resource : rlimit, getrlimit, setrlimit, RLIMIT_NOFILE;
1807+
import core.sys.posix.sys.resource : rlimit, rlim_t, getrlimit, setrlimit, RLIMIT_NOFILE;
18051808

1806-
// Save current limit
1807-
rlimit originalLimit;
1808-
if (getrlimit(RLIMIT_NOFILE, &originalLimit) != 0)
1809-
return; // Can't test if we can't get the limit
1809+
// This test only applies on platforms where rlim_t can exceed int.max.
1810+
static if (rlim_t.sizeof > int.sizeof)
1811+
{
1812+
// Save current limit
1813+
rlimit originalLimit;
1814+
if (getrlimit(RLIMIT_NOFILE, &originalLimit) != 0)
1815+
return; // Can't test if we can't get the limit
18101816

1811-
// Set RLIMIT_NOFILE to a value that overflows int (> int.max)
1812-
rlimit highLimit;
1813-
highLimit.rlim_cur = cast(ulong) int.max + 1;
1814-
highLimit.rlim_max = originalLimit.rlim_max;
1817+
// Set RLIMIT_NOFILE to a value that overflows int (> int.max)
1818+
rlimit highLimit;
1819+
highLimit.rlim_cur = cast(rlim_t) int.max + 1;
1820+
highLimit.rlim_max = originalLimit.rlim_max;
18151821

1816-
// If we can't raise the limit (e.g. no permission), try with rlim_max
1817-
if (setrlimit(RLIMIT_NOFILE, &highLimit) != 0)
1818-
{
1819-
highLimit.rlim_cur = originalLimit.rlim_max;
1820-
if (highLimit.rlim_cur <= int.max)
1821-
return; // Can't set a high enough limit to test the overflow
1822+
// If we can't raise the limit (e.g. no permission), try with rlim_max
18221823
if (setrlimit(RLIMIT_NOFILE, &highLimit) != 0)
1823-
return;
1824-
}
1825-
scope(exit) setrlimit(RLIMIT_NOFILE, &originalLimit);
1824+
{
1825+
highLimit.rlim_cur = originalLimit.rlim_max;
1826+
if (highLimit.rlim_cur <= int.max)
1827+
return; // Can't set a high enough limit to test the overflow
1828+
if (setrlimit(RLIMIT_NOFILE, &highLimit) != 0)
1829+
return;
1830+
}
1831+
scope(exit) setrlimit(RLIMIT_NOFILE, &originalLimit);
18261832

1827-
// This should not throw "Failed to allocate memory"
1828-
TestScript prog = "exit 0";
1829-
assert(execute(prog.path).status == 0);
1833+
// This should not throw "Failed to allocate memory"
1834+
TestScript prog = "exit 0";
1835+
assert(execute(prog.path).status == 0);
1836+
}
18301837
}
18311838

18321839
@system unittest // Environment variables in spawnProcess().

0 commit comments

Comments
 (0)