[WIP] Cleanly stop() loop when awaiting promise without fiber

[clue]

This changeset makes sure we cleanly stop() the loop when awaiting a promise without a fiber. This is especially important for test suites and other use cases where you would mix await() and Loop::run() calls. The await() call may currently suspend the Loop::run() call and leave it in an unpredictable state, possibly leaving socket resources or timers behind.

Builds on top of #15 and #32
Refs #27 and #50

[WyriHaximus]

This is going to be a fun one to test out before we can ship it to production 😅

[WyriHaximus]

Interestingly enough I'm running into this while making the reactphp/socket#302 passes before opening it up for review. Will check our non-test environments and report back.

[WyriHaximus]

Deployed it in a side project to see how it goes, after inspecting the code it shouldn't be an issue but I want to see it running for a while without causing issues first 😅

[tomas-novotny]

Hello, hows the testing going @WyriHaximus, is it ready for merging? It would helped us with fixing test suites for react/async: ^4.0.

[clue]

It would helped us with fixing test suites for react/async: ^4.0.

@tomas-novotny I'm curious, can you give some details what specific use case this would help with? I consider this change a WIP that could potentially have a significant impact (see #27 for context), so I would like to get a better understanding how this would affect the larger ecosystem.

[tomas-novotny]

@clue I tried to come up with a simplified version of our tests that were failing without this change, but I was unable to do so.

On further investigation, I found that ^4.0 behaves a little differently than ^3.0, but actually better, so the problem was our wrong test.

So after fixing a few assertions and calling cancelTimer everything was fixed and now it passes our tests.

[WyriHaximus]

@tomas-novotny Well actually, I've been running this in production on a low traffic project for nearly 4 months without issues. As far as I know because I do have a bunch of odd pod restarts but those could also have different reasons for restarting.

[tomas-novotny]

I managed to create some test cases that fail with reactphp/async:4.0.0.

https://gist.github.com/tomas-novotny/5abdbd4474c25e768c6eeb168c32cfc1

• The testPublishConsume[1/2/3] test cases pass when you run them individually, but when you run them all, only the first one passes and the rest fail with various errors.

• The testSyncPublishWithAsyncConsume always fails.

These PR changes fix these tests. I have not been able to fix them any other way.

Is there something I am doing wrong or some change that will make it work without these PR changes?

[clue]

@tomas-novotny Thank you for looking into this and providing reproducible test scripts! Definitely a good start, though I would love to see if we could eventually trim this down to remove any external dependencies to get to the core of this.

The gist here seems to be that your test suite mixes await() and Loop::run() calls which usually should be avoided. This PR here ensures that await() cleanly stops the loop so a following Loop::run() works without issues, but the recommendation to not mix both styles still seems reasonable in either case. We're working on making the Loop opaque in a future ReactPHP v3 (https://github.com/orgs/reactphp/discussions/481), so I wonder what impact this would have on your test cases.

[tomas-novotny]

@clue Thank you for your hints how to fix my test suites.
I replaced Loop:run() with awaiting for a deferred promise and it works fine even without this PR changes.

+$deferred = new Deferred(); // failsafe to stop loop $timers[] = $loop->addTimer(5, static function () use ($loop): void{- $loop->stop();+ $deferred->reject(); }); // periodic check if all messages have been consumed $timers[] = $loop->addPeriodicTimer(1, static function (TimerInterface $timer) use ($loop, $messages, &$consumedMessages): void{if (count($messages) !== count($consumedMessages)){return} $loop->cancelTimer($timer); - $loop->stop();+ $deferred->resolve(); }); // finish consuming await($consumer); // run loop -$loop->run();+await($deferred->promise());