Open durban opened 7 months ago
I rewrote the test to be slightly more explicit:
"foobar" in real {
IO(new AtomicInteger) flatMap { ctr =>
val test = IO.deferred[Unit] flatMap { latch =>
val t = IO uncancelable { poll =>
latch.complete(()).uncancelable *> poll {
IO.async_[Unit] { cb =>
ctr.getAndIncrement()
cb(Right(()))
}
}
}
t.start flatMap { fib =>
latch.get *> fib.cancel *> fib.joinWithUnit
}
}
val N = 100
test.replicateA_(N) flatMap { _ =>
IO(ctr.get()) flatMap { count =>
IO(count mustEqual N)
}
}
}
}
I think this also reveals where the problem is: in the partial expression *> IO.async_
, cancelation is checked twice, and only the first of them is suppressed. The "uncancelable
region extension" semantic isn't really an extension of the region so much as an active suppression of a very specific cancelation check: the one at the end of the uncancelable
. All IO
stages check cancelation at the very start of their execution. Originally, uncancelable
checked additionally at the end of its execution (when its continuation is popped). This check was inserted to comply with the functor laws (since FlatMap
and Map
both check at the front as well), but it also makes it impossible to have a clean seal in the construction poll(uncancelable(_ => ...) /*here*/)
(since the extra, now suppressed, check would happen at the inner )
).
So what does this have to do with us here? Well, async_
checks for cancelation at the beginning of its execution, just like everything else. Does that make it cancelable? I don't think so? It's cancelable in the same sense that IO.never.uncancelable.start.flatMap(_.cancel)
might succeed, despite the uncancelable
, since cancelation can always prevent the uncancelable thing from ever running in the first place, which is what's happening here.
So I think this isn't a bug.
I think the MonadCancel
scaladoc is misleading then. It has the example:
F.uncancelable(poll => foo(poll)).flatMap(f)
It is guaranteed that we will not observe cancelation after
uncancelable
and henceflatMap(f)
will be invoked.
Then it talks about Resource#allocated
. My test is not really that different from
Resource.eval(IO.unit).allocated.flatMap { x => IO.delay(...) }
And we're getting cancelled after allocated
. I think I understand why, but the MonadCancel
scaladoc seems to tell something else. But to get what we want, I think we need an additional, outer uncancelable
; the scaladoc doesn't talk about that.
Besides all this, in async_
there is an additional cancelation check: between IOCont
and body
. It doesn't really matter, because (as you say), we can get cancelled before IOCont
, but it's there.
I think the MonadCancel scaladoc is misleading then. It has the example:
Yeah I think this would be worth updating. The scaladoc isn't inaccurate (it says the flatMap
will be invoked, but it doesn't say anything about the effect produced by the f
), but it's unquestionably confusing.
And we're getting cancelled after allocated. I think I understand why, but the MonadCancel scaladoc seems to tell something else. But to get what we want, I think we need an additional, outer uncancelable; the scaladoc doesn't talk about that.
Yeah this is a common pattern. In general, the way to think about it is that you shouldn't assume anything about the trailing flatMap
semantic. That is, in effect, just an implementation detail. What you can assume is that uncancelable(poll => poll(uncancelable(_ => ...)))
has no gaps in coverage.
Yeah, okay, I'm reopening and making this a docs issue.
What you can assume is that
uncancelable(poll => poll(uncancelable(_ => ...)))
has no gaps in coverage.
No gaps at the end. There is a gap poll(/*here*/...)
(see #3553).
This test fails:
I believe this shows that
IO.async_
is cancelable: waiting onlatch
ensures, that the fiber is not cancelled before even starting; and the boundary afteruncancelable
(i.e., right beforeasync_
) is non-cancelable. While it was removed from theasync_
scaladoc that it is uncancelable (by #3091), I don't think this is intentional. I think it's intended to be uncancelable.A similar case is observable if
async
returnsNone
(i.e., no finalizer). And also forasyncCheckAttempt
, and even if it returnsRight(...)
.I'm not sure about the impact of this, as they seem to be cancelled before registration (i.e., between
IOCont
andbody
). But this seems like a bug.