Change async_ to be uncancelable

[djspiewak]

This PR fundamentally changes the semantics of async_ and async with respect to nullary finalizers. Specifically, async_'s only mode of operation, which is equivalent to when async produces None from its registration action. For example:

val foo = IO.async_(cb => thingy.register(cb))

// equivalently

val foo = IO async { cb =>
  IO {
    thingy.register(cb)
    None
  }
}

Previously, this corresponded to a semantic in which foo was cancelable and did not activate a finalizer, similar to flatMap (and indeed, most effect stages). Unfortunately, this was also a form of backpressure corruption and, in some cases, a resource leak. For example, foo above is not in fact cancelable since we have provided no mechanism for removing the callback from the map. Treating it as cancelable would be a form of resource leak, since cb would be orphaned within the thingy register.

The correct way to avoid this situation is for thingy to define an unregister method (as most such mechanisms do in practice):

val foo = IO async { cb =>
  IO {
    thingy.register(cb)
    Some(IO(thingy.unregister(cb)))
  }
}

With this adjustment, foo can be safely canceled, and when we do so it will remove cb from thingy, cleaning up and backpressuring the canceling fiber.

This PR changes the semantics of async when None is produced to be uncancelable. Since no finalizer has been provided, the asynchronous effect is now considered to be uninterruptible, similar to how delay cannot be canceled for a similar reason. Thus, any async which returns None, any asyncCheckAttempt which returns Left(None), and all async_ effects are now uncancelable.

In most cases, the ideal way to adapt to this change is to simply provide a finalizer for your asynchronous constructor by producing Some instead of None. When this is not possible, the uncancelability is likely to be the ideal default for avoiding memory leaks and retaining backpressure. In the rare cases where the previous semantics are desired, it is possible to restore them by producing Some(IO.unit) as a finalizer. One such example is the never effect:

val never: IO[Nothing] = IO.async(_ => IO.pure(Some(IO.unit))))

Previously, never was defined as IO.async(_ => IO.pure(None))), but as that formulation is now uncancelable, it must be rewritten as shown above. In theory, this is a more explicit version, since it correctly expresses the fact that never is cancelable and no actions need to be taken to release resources when it is canceled.

Relates to #3199

[djspiewak]

Published as 3.5-4ba2590

@armanbilge we should try to canary this around the ecosystem to see what impact it has.

[armanbilge]

Well, it broke broke fs2-io JVM/Native.

==> X fs2.io.net.tcp.SocketSuite.tcp - echo requests - each concurrent client gets back what it sent  31.003s java.util.concurrent.TimeoutException: test timed out after 31 seconds
==> X fs2.io.net.tcp.SocketSuite.tcp - readN yields chunks of the requested size  31.0s java.util.concurrent.TimeoutException: test timed out after 31 seconds
==> X fs2.io.net.tcp.SocketSuite.tcp - write - concurrent calls do not cause a WritePendingException  31.0s java.util.concurrent.TimeoutException: test timed out after 31 seconds
==> X fs2.io.net.tcp.SocketSuite.tcp - addresses - should match across client and server sockets  31.001s java.util.concurrent.TimeoutException: test timed out after 31 seconds
==> X fs2.io.net.tcp.SocketSuite.tcp - options - should work with socket options  31.001s java.util.concurrent.TimeoutException: test timed out after 31 seconds
==> X fs2.io.net.tls.TLSSocketSuite.TLSSocket - echo  31.0s java.util.concurrent.TimeoutException: test timed out after 31 seconds
==> X fs2.io.net.tls.TLSSocketSuite.TLSSocket - error  31.001s java.util.concurrent.TimeoutException: test timed out after 31 seconds
==> X fs2.io.net.tls.TLSSocketSuite.TLSSocket - echo insecure client with Endpoint verification  31.0s java.util.concurrent.TimeoutException: test timed out after 31 seconds

And also fs2-io Node.js.

==> X fs2.io.IoPlatformSuite.unacknowledged 'end' does not prevent writeWritable cancelation 31.00s java.util.concurrent.TimeoutException: test timed out after 31 seconds

It will probably break browser JS stuff too.

[djspiewak]

New snapshot: 3.5-4b87497

[armanbilge]

This change definitely breaks some stuff downstream, but in my experiments it's only broken "FFI" code using async_, and these were things I expected to break, precisely because I didn't understand how they were working in the first place (which is how I stumbled on this issue).

Since cancelation is not the happy-path, I expect there will be other downstream issues that won't surface in happy-path-focused test suites.

Nonetheless, personally I am eager to move forward with this.

[djspiewak]

@armanbilge How far did we canary this? Just Fs2, or were we able to try it out in further-downstream things as well?

[armanbilge]

I also tried it out in my browser integration projects (fs2-dom etc.) but not further than that, since those projects are where all the async_s live.

I suspect blaze will get hit by this. Maybe worth opening an issue there.

[djspiewak]

I think we need a snapshot of fs2 which contains this change in order to move forward with the ecosystem canary.

[armanbilge]

I put up an FS2 PR in typelevel/fs2#3063 and can publish a snapshot of that after I rig my faraday cage 😇