-
-
Notifications
You must be signed in to change notification settings - Fork 745
std.process: Retry interrupted syscalls when creating processes #10885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for your pull request, @CyberShadow! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + phobos#10885" |
version (unittest_eintr)
version (Posix)
@system unittest
{
// This stress test verifies that spawnProcess correctly handles EINTR
// (Interrupted system call) errors by retrying the system calls.
// We flood the process with signals to reliably trigger EINTR.
//
// To run this test:
// rdmd --main -unittest -version=unittest_eintr std/process.d
import core.sys.posix.signal : sigaction, sigaction_t, SIGALRM, SA_RESTART;
import core.sys.posix.sys.time : itimerval, setitimer, ITIMER_REAL;
import core.sys.posix.unistd : alarm;
import std.stdio : writeln;
// Install a dummy signal handler for SIGALRM
sigaction_t sa;
sa.sa_handler = (int sig) {}; // Empty handler
sa.sa_flags = 0; // Explicitly no SA_RESTART - we want EINTR
sigemptyset(&sa.sa_mask);
sigaction(SIGALRM, &sa, null);
// Set up interval timer to fire very frequently (every 100 microseconds)
// This should cause frequent interruptions of system calls
itimerval timer;
timer.it_interval.tv_sec = 0;
timer.it_interval.tv_usec = 100; // 100 microseconds
timer.it_value.tv_sec = 0;
timer.it_value.tv_usec = 100;
setitimer(ITIMER_REAL, &timer, null);
scope(exit)
{
// Stop the timer
itimerval stopTimer;
stopTimer.it_interval.tv_sec = 0;
stopTimer.it_interval.tv_usec = 0;
stopTimer.it_value.tv_sec = 0;
stopTimer.it_value.tv_usec = 0;
setitimer(ITIMER_REAL, &stopTimer, null);
}
// Spawn processes repeatedly while being bombarded with signals
// This exercises the EINTR retry logic in read() and waitpid()
writeln("Running EINTR stress test (spawning 100 processes with signal flooding)...");
foreach (i; 0 .. 100)
{
auto pid = spawnProcess(["cat", "/dev/null"]);
auto exitCode = wait(pid);
assert(exitCode == 0, "Process failed");
// Note: execute() is not tested here because it reads pipe output
// through std.stdio.File.byChunk, which has its own EINTR issues
}
writeln("EINTR stress test passed!");
}Not sure if it should be included, it might be too finicky to run it in all CIs, and leaving it versioned out is just adding dead code. |
|
Maybe instead of a |
|
Hmm, it looks like Phobos doesn't really have a stand-alone test suite set up like Druntime has. Right now
I guess we can keep adding sections like the betterC test but it's not going to be as clean as the Druntime approach. Not sure if it's worth going any further just for the sake of this fix. |
|
Hah, I did similar to my process thing just last week: adamdruppe/arsd@b3d88ca#diff-a1484c902f60d0956e4cf2db3fb30e1fd4a6ff352996f4415f7550fbad36ca17R8358 Hit it when trying to do background/foreground implementation of that little unix shell i wrote last weekend which triggered EINTR pretty reliably since the shell tries to do job control right after the forks when creating pipelines. Can't assume retry is the right thing in general, but in this case i certainly agree it is. I don't have an approve button here, but if i did, i'd click it. |
Fixes #10884