Changes to `async_` cancelation semantics
armanbilge opened this issue · 6 comments
I let CI try this in armanbilge@df420f2 and there are some test failures.
[error] Failed tests:
[error] org.http4s.blaze.client.PoolManagerSuite
[error] org.http4s.blaze.client.ClientTimeoutSuite
[error] org.http4s.blaze.client.BlazeClientSuite
Release candidate is available. See "major changes" section in release notes.
Most of the async_ calls don't have obvious resources to release. There's the stage, but shutdown of the stage is probably why we're being canceled. My two kneejerk reactions are the Some(F.unit).pure[F] that's generally discouraged, or shutting down the stage again just in case. I'm just not sure.
Some(F.unit).pure[F] seems strange, sometimes what I've seen is F.delay(setupCallback()).as(Some(F.unit))
Edit: well, maybe just a case of potato-potato 😋
Oh, right. async_ hides the delay of setting up the callback. It should look more like your version if we go that route.
Addressed in #806