Improve MonadCancel scaladoc

[durban]

This test fails:

      "foobar" in real {
        IO(new AtomicInteger).flatMap { ctr =>
          val test = IO.deferred[Unit].flatMap { latch =>
            val t = latch.complete(()).uncancelable *> 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 believe this shows that IO.async_ is cancelable: waiting on latch ensures, that the fiber is not cancelled before even starting; and the boundary after uncancelable (i.e., right before async_) is non-cancelable. While it was removed from the async_ 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 returns None (i.e., no finalizer). And also for asyncCheckAttempt, and even if it returns Right(...).

I'm not sure about the impact of this, as they seem to be cancelled before registration (i.e., between IOCont and body). But this seems like a bug.

[djspiewak]

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.

[durban]

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 hence flatMap(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.

[djspiewak]

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.

[durban]

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).