Skip to content

Conversation

@CyberShadow
Copy link
Member

Fixes #10884

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @CyberShadow!

Bugzilla references

Your 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 locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#10885"

@CyberShadow
Copy link
Member Author

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.

@pbackus
Copy link
Contributor

pbackus commented Oct 25, 2025

Maybe instead of a unittest block, it could be a script in the test/ directory?

@CyberShadow
Copy link
Member Author

CyberShadow commented Oct 25, 2025

Hmm, it looks like Phobos doesn't really have a stand-alone test suite set up like Druntime has. Right now test/ contains:

  • two Dub tests invoked directly from .circleci/run.sh, bypassing the makefile
  • one betterC test which has its own section in the makefile

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.

@adamdruppe
Copy link
Contributor

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.

@thewilsonator thewilsonator merged commit 5ae986d into dlang:master Oct 27, 2025
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Could not read from pipe to get child status (Interrupted system call)

5 participants