Glad to see that you consider using Fibers! 🤗
Just some thoughts about this:

• What if the fiber is never resumed?
• What if the fiber is resumed with throw?
• What if the fiber is resumed without calling usleep in the meantime? (To be honest, I'm not sure Process actually needs this…)

My point is that it could be interesting to define a contract about "how to use safely a SF Process in a Fiber".
A part of this contract could also be transmitted via the suspend function, a kind of promise to be resumed in n microseconds, or when a goal is reached… 🤷

I have no particular idea about this, I'm not sure if Symfony needs a complete async tools set, but I'm just wondering if this suspend can be an open door to wrong usages.

Anyway, this PR has my full attention 😉👍

Glad to see that you consider using Fibers!

Thanks a lot for your feedback

• What if the fiber is never resumed?

The process will last, but it's exactly the same as when you write the follow code

$p = new Process['sleep', '10'];
$p->start();
// Nothing here

So IMHO, nothing has to be done here

• What if the fiber is resumed with throw?

What would be the use case to resume it with throw?

• What if the fiber is resumed without calling usleep in the meantime? (To be honest, I'm not sure Process actually needs this…)

Indeed, I was not sure about the use case, but actually, it eases the async handling of process.

My point is that it could be interesting to define a contract about "how to use safely a SF Process in a Fiber".

I totally agree with that 👍🏼, and more generally in Symfony

I'm just wondering if this suspend can be an open door to wrong usages.

Yes, it can. I said that, because we keeps reading issue where people do really strange things :) I guess it's how OSS works :)

The process will last, but it's exactly the same as when you write the follow code [...]
So IMHO, nothing has to be done here

👍

What would be the use case to resume it with throw?

Since it's possible, we can expect that someone will try 😅.
But it could a way to stop a too long process (timeout), or to cancel it for other external reasons (too much opened fibers, too much memory... 🤷‍♂️)

Yes, it can. I said that, because we keeps reading issue where people do really strange things :) I guess it's how OSS works :)

😅😉

What's the status here? @lyrixx?

I think there is consensus yet on how to add fibrer support in Symfony. Let's close it

But if someone want it, we could also merge it.

mhh wait why is this not merged?

Wep, too bad. People are not ready for that!

And we need that in castor

cc @joelwurtz

@lyrixx Not sure how you came to that conclusion as you were the one closing this PR.

Let's re-open the PR 👍🏼 (rebased, and updated)

After playing a bit with the process and fiber, i think there is no need to add fiber support in this class, the only things the process class need would be getter for the stdout and stderr stream.

Here is an example on how it may work (i use closure binding to get the streams) :

require 'vendor/autoload.php';

$pid = null;
$running = true;

// listen for sigchld
pcntl_signal(SIGCHLD, function ($signo, $data) use (&$pid, &$running) {
    if ($pid === $data['pid']) {
        $running = false;
    }
});

pcntl_async_signals(true);

$fiber = new Fiber(function () {
    $process = \Symfony\Component\Process\Process::fromShellCommandline('echo -n "test"; echo -n "bar" 1>&2 ; sleep 2');

    // bind a function to get the output stream
    $getStdout = Closure::bind(function () {
        return $this->stdout;
    }, $process, $process);
    $getStderr = Closure::bind(function () {
        return $this->stderr;
    }, $process, $process);

    $process->start();

    $stdout = $getStdout();
    $stderr = $getStderr();

    stream_set_blocking($stdout, 0);
    stream_set_blocking($stderr, 0);

    while ($process->isRunning()) {
        $action = \Fiber::suspend([$stdout, $stderr]);

        if ($action === 'stdout') {
            $content = $process->getIncrementalOutput();

            if ($content !== '') {
                var_dump('stdout : ' . $content);
            }
        }

        if ($action === 'stderr') {
            $content = $process->getIncrementalErrorOutput();

            if ($content !== '') {
                var_dump('stderr : ' . $content);
            }
        }
    }

    return $process->getExitCode();
});

[$stdout, $stderr] = $fiber->start();
$streams = [$stdout, $stderr];

while (!$fiber->isTerminated()) {
    $write = null;
    $except = null;

    $read = $streams;
    $modified = stream_select($read, $write, $except, 0, 100_000);

    if ($modified === false) {
        break;
    }

    if ($modified > 0) {
        foreach ($read as $stream) {
            if ($fiber->isTerminated()) {
                break;
            }

            if ($stream === $stdout) {
                $fiber->resume('stdout');
            }

            if ($stream === $stderr) {
                $fiber->resume('stderr');
            }
        }
    }
}

var_dump('exit code : ' . $fiber->getReturn());

There may be some adjustement to do in the process class, but checking for fibers may not be needed IMHO

Also this totally remove sleep inside the process, (it may sleep but it depends only on the event loop implementation)

Exposing those streams make it be compatible with Revolt also, and i believe it would be better to provider a helper around the Process class that make it compatible with revolt ?

Didn't see this was in the wait function, it can make sense for it for existing library / app that use this method.

Part of this example can be reuse, like instead of doing a sleep it could do a stream_select with a timeout so it wakes up as soon as there is something to read / write or use the SIGCHLD signal to wake up also when the process has stopped (only with pcntl extension) ?

Is it still relevant?