search

reactphp / async

Async utilities and fibers for ReactPHP.
https://reactphp.org/async/
MIT License
204 stars 19 forks source link

[4.x] Forward compatibility with upcoming Promise v3 #48

Closed clue closed 2 years ago

clue commented 2 years ago

Marking this as WIP as the tests currently fail with Promise v3. I've marked the affected tests as incomplete for now, but before merging, the root cause should be fixed instead. Solved via https://github.com/reactphp/promise/pull/229.

Builds on top of https://github.com/reactphp/promise/pull/75 and https://github.com/reactphp/promise/pull/213 and https://github.com/reactphp/promise-timer/pull/54 Changes cherry-picked from #47 for 3.x, but also refs cancellation of async() (see #20 and #42) Refs #43

clue commented 2 years ago

I've updated this to show that this is currently only blocked by https://github.com/reactphp/promise/pull/229. The gist is that we suspend fibers when a promise settles which could previously cause some invalid state in internals of promises to break and not execute any other callbacks. This is now addressed upstream for Promise v3 as this is the only version that shows this problem.

As an alternative, I've also looked into addressing this in this component instead, but this essentially requires suspending the fiber outside of the promise callbacks using a future loop tick. This would essentially revert the major improvements we've recently added via #32 and would thus cause regressions for #27.

As such, I believe addressing this in https://github.com/reactphp/promise/pull/229 is the best way moving forward. The resulting patch looks surprisingly straight-forward, but you're looking at several days worth of work on investigating this root cause and coming up with this minimal solution. Enjoy!

clue commented 2 years ago

Updated PR now that https://github.com/reactphp/promise/pull/229 has been merged, this is now ready to be shipped :shipit:

clue commented 2 years ago

Rebased to resolve merge conflict now that #55 is in. This also happens to remove the dependency on PromiseTimer, so this no longer refs https://github.com/reactphp/promise-timer/pull/54 :shipit: