reactphp / promise

Promises/A implementation for PHP.
https://reactphp.org/promise/
MIT License
2.38k stars 146 forks source link

Continue internal `Queue` execution also after fiber is suspended #229

Closed clue closed 2 years ago

clue commented 2 years ago

This simple changeset updates the internal Queue class to continue execution also after a fiber is suspended (PHP 8.1+). In particular, this is needed to make our new Async v4 package compatible with Promise v3 (https://github.com/reactphp/async/pull/48).

The added test cases are the gist of why our Async integration currently fails with Promise v3 only (https://github.com/reactphp/async/pull/48), but other than that I agree that they might look a bit weird. I don't particularly care about how an individual promise behaves if a fiber is suspended (perfectly reasonable to argue in either direction), but the original version has some severe problems as all promises start to fail once a fiber is suspended.

This is the minimal changeset I could come up with, but I understand that this reverts parts of the iterative logic introduced in #28/#82/#86. The test suite confirms my changes do not break any of the known assumptions and in fact bring the execution order more in line with how Promise v2 and v1 work (this causes the build error in https://github.com/friends-of-reactphp/mysql/pull/157 and likely also in https://github.com/reactphp/socket/pull/214). I would suggest we should look into this iterative logic and its consequences again in a follow-up PR if we see a need for this. Until we can come up with more specific test cases that highlight now the original iterative logic is needed, I would argue that merging this as is to move forward with https://github.com/reactphp/async/pull/48 seems reasonable.