typelevel / cats-effect-cps

An incubator project for async/await syntax support for Cats Effect
Apache License 2.0
94 stars 12 forks source link

make resulting effect cancellable again #192

Open cornerman opened 9 months ago

cornerman commented 9 months ago

Currently, any effect created by the async macro is not cancelable. This PR fixes that.

Probably a regression after the semantics of async_/async changed in regards to cancellability.

I have also fixed the test for "be cancellable" - which was not asserting correctly.

cornerman commented 9 months ago

Somehow the "be cancellable" test is now flaky - I could make it pass reliably with this change: https://github.com/typelevel/cats-effect-cps/pull/192/commits/99e8dbf1171dd97b31bd0abc4809d61ab5bacfe4.

But this looks suspicious to me, maybe there is something else going on?

cornerman commented 9 months ago

I have added a potential fix, which makes the test pass without the above mentioned workaround. See: https://github.com/typelevel/cats-effect-cps/pull/192/commits/c4e98a8940581e87fc647401fcd029a386449896

There, I have introduced a stopSignal to cancel tasks running in the dispatcher, when the actual effect is cancelled. Please let me know, if this makes sense or whether there is a better fix. From what I can see, it does solve the issue at hand.