Can someone explain to me why `AcquireStep` needs ...

# arrow-contributors

[Youssef Shoaib [MOD]]

Can someone explain to me why AcquireStep needs to exist? I'm not seeing the issue with bind being called inside the acquire. What would be the issue with that? The resource that was bound inside will have its finalizer also updated and everything, and as long as it completes successfully, the current finalizer will also be added to the list. Is there something I'm missing?

[simon.vergauwen]

Hey Youssef,
Good question!

[simon.vergauwen]

I've been wondering for some time, but since it was never requested to change I left it this way.

This was originally inspired by ManagedT from Haskell, and Resource from Cats-effect. The problem is a compositional one, the acquire step is uncancellable, since otherwise we can break the composed function.

See https://github.com/typelevel/cats-effect/issues/382.

However, I am not longer entirely sure it's required in Kotlin. In Kotlin our resource will almost never be build out of IO with flatMap, and KotlinX doesn't automatically add cancellation boundaries. So I see this as less problematic.

Some scenario could be where someone adds a Schedule around the acquire step, but then I am not sure why it should not be cancelled. There are most likely not going to be cancellation boundaries in the "setup" part of the acquisition.

As a matter of fact we purpose to keep the acquire step small.

cc\\ @Alexandru Nedelcu you were part of a lot of these discussions in Scala, I would love your thoughts here! 🙏 (Sorry for the random tag).

Also calling @Alejandro Serrano.Mena, not sure how familiar you are with ManagedT and how it works with IO in relation to cancellation/interruption but I'd love your thoughts as well.

PS: Great work in the PR @Youssef Shoaib [MOD], but this change is something that I rather wait with until after KotlinConf so I can properly wrap my head around it 😅

[Alexandru Nedelcu]

@simon.vergauwen acquire should be uncancellable by default, however in Cats-Effect 3, the developer does have a builder to describe cancellation logic for acquire, as well. This would be needed, for example, in the case of a "blocking queue" or a "semaphore", where the waiting part should be cancellable.

Kotlin does not have cancellation boundaries, except when it does 😉
For example:

val queue = new ArrayBlockingQueue[Resource]()

// acquisition
val res = runInterruptible {
  queue.take()
} // <- cancellation boundary here
try {
  doStuff(res)
} finally {
  // release
  queue.offer(res)
}

Unfortunately, this can lead to a leak, as Kotlin does in fact introduce an implicit cancellation boundary.

[simon.vergauwen]

Hey @Alexandru Nedelcu,
Thanks for the feedback, that is indeed how I understood the rationale for acquire behavior from CE.

Kotlin does not have cancellation boundaries, except when it does 😉

Good point about runInterruptible because, NonCancellable doesn't do that.

withContext(NonCancellable) {

} // <-- no cancellation boundary

Which makes it even more tricky I guess, since this is fine:

val acquired = withContext(NonCancellable) {
  acquire()
} // <-- no cancellation boundary
closing { e: Throwable? -> acquired.close(e) }

the developer does have a builder to describe cancellation logic for acquire

Hmm, I think I missed that. Is the new type for acquire Poll[F] => A instead of => A? There is a Poll implementation for Kotlin, which is still useful in special scenarios like this.

I'm wondering if we need to rethink the primitives here for Kotlin to better match KotlinX Coroutines cooperative behavior, but operators like runInterruptible indeed make things trickier.

[Alexandru Nedelcu]

Yes, I'm talking of Poll[F] => A , which is part of the uncancellable operator that's now the primitive. For Cats-Effect, it works out well.

If you make acquire non-cancellable, that's good enough for most cases. Again, we're talking about modeling a resource whose acquisition can be interrupted. And there are some use-cases for that, such as getting a resource from a resource-pool.

The runInterruptible example can be fixed by capturing the resource in a var.

val queue = new ArrayBlockingQueue[Connection]()

var res: Connection? = null
try {
  // acquisition  
  runInterruptible {
    res = queue.take()
  }
  // usage part
  use(res)
} finally {
  // release
  if (res != null) queue.offer(res)
}

[Alexandru Nedelcu]

So, given the above example, maybe you can expose some API that can basically do that.

[Alexandru Nedelcu]

One way of doing it would be to inject a callback in the acquisition function, that should be called when the resource is ready.

[Youssef Shoaib [MOD]]

Would that callback not simply just take the equivalent of the release lambda passed to install? Perhaps the key thing here is that one should just onRelease {...} right when a resource is ready

[simon.vergauwen]

Indeed, this would be correct.

val queue = new ArrayBlockingQueue[Connection]()
val res = runInterruptible {
val res = queue.take().also { res ->
  onRelease { _: Throwable? -> queue.offer(res) }
}
use(res)

There is no cancellation boundary between take and onRelease here.

[Alexandru Nedelcu]

I never tried doing that with Arrow. Does that code compile? I'm trying to test it, but the onRelease function I found is a suspending function, and one problem would be that the function passed in runInterruptible is not suspending.

[simon.vergauwen]

This specific change is being worked on in 2.x.x, but changing this kind of stuff is dangerous

[Alexandru Nedelcu]

Personally, whenever Cats-Effect broke compatibility, I hated it for it, even if I liked how the API behaved afterwards 🙂

[simon.vergauwen]

onRelease as Youssef proposed in a Resource improvement, https://github.com/arrow-kt/arrow/pull/3431 is no longer suspended.

I'm considering a more generalised DSL, https://github.com/arrow-kt/arrow/pull/3443, but it comes with some other issues like what if I want to close resources, but keep other finalisers untouched. This introduces a new kernel DSL module, and leaves the original Fx module untouched. Avoiding breaking compatibility as well 😉

[simon.vergauwen]

My idea is that when Context Parameters land, we need to release 3.0, and the only breaking change would be binary, and in RaiseAccumulate. Since extension receiver can be refactored to context parameter in a binary compat way, although not sure if we have to with single receivers.

This way we can split off the DSL, and leave everything as is. The only thing it might require additionally is imports.., which could be automatically inserted by the IntellIJ plugin, but it still breaks compatibility.

> Personally, whenever Cats-Effect broke compatibility, I hated it for it
I wonder if I should just stop, and leave Arrow as is to avoid breaking compatibility... I understand this sentiment 😅

[Alexandru Nedelcu]

There's a constant tension between wanting to not breaking people's code, versus wanting correctness. It's hell on earth for library developers 🙂

[Alexandru Nedelcu]

Btw, IMO, it's useful for the release to be able to suspend, although the default should be non-cancellable, and in Cats-Effect finalizers are always non-cancellable.

[simon.vergauwen]

Btw, the onRelease is non-suspend, but the lambda is suspend.

[simon.vergauwen]

onRelease is essentially just registering the lambda into an AtomicReference.

[Alexandru Nedelcu]

In 2.x.x?

[Alexandru Nedelcu]

So, right now with the stable release, is there any way to create a Resource with a cancellable acquisition?

[simon.vergauwen]

In the existing release, no I don't think so. It's old CE1 behavior

[simon.vergauwen]

The entire acquire is wrapped in withContext(NonCancellable) { }

[Alexandru Nedelcu]

Right, it's a good default, but sometimes you need more 🙂

[simon.vergauwen]

Exactly, still wondering where the right balance lies

[simon.vergauwen]

Or just more Kotlin idiomatic naming for resource(acquire, release) and an interruptible variant.

[Alexandru Nedelcu]

You could do what Cats-Effect is doing, which is to add extra builders. Like, there's a builder that has a non-cancellable acquisition, and then there's the builder that gives you Poll, for more control.

[Alexandru Nedelcu]

So not necessarily the same API, but it could have a builder for grown-ups that know what they are doing

[simon.vergauwen]

Okay, guess I need to read up on some PRs in CE3 😅

[Alexandru Nedelcu]

So Resource.make , Resource.apply or Resource.makeCase from Cats-Effect are very much like the current install in Arrow. Non-cancellable, and does the right thing in 80%+ of cases.

Then you have Resource.makeFull , Resource.makeCaseFull , and Resource.applyFull giving you Poll for more control.

[Alexandru Nedelcu]

You could do something similar, IMO, like installFull or something.

[Alexandru Nedelcu]

I mean, not necessarily the same API, but have a separate builder that gives you a non-cancellable acquisition, and you're on your own to install the onRelease at precisely the right time.

[simon.vergauwen]

This is a KotlinX implementation for Poll, https://github.com/nomisRev/arrow-fx-coroutines-utils/blob/main/src/commonMain/kotlin/io/github/nomisrev/UncancellableRegion.kt.

but I'm not too excited about the naming, or the APIs 😅

[Alexandru Nedelcu]

Cats-Effect replaces the language's call-stack, so it makes a lot of sense.

In Kotlin, the use of the stack and of normal statements such as try/catch, helps a lot, except in cases such as runInterruptible which can catch one by surprise. I think runInterruptible can be fixed, but isn't due to backwards-compatibility concerns. (I remember seeing an issue open for it, but can't find it now)

[Youssef Shoaib [MOD]]

Btw I've restarted the aforementioned PR , and have found and fixed some edge cases and undocumented and unintentional behaviours. https://github.com/arrow-kt/arrow/pull/3518

[Youssef Shoaib [MOD]]

With these refactorings, some clear patterns emerge. For instance, install(acquire, release) is just:

val a = withContext(NonCancellable) { acquire() }
onRelease { release(a, it) }
onRelease { release(a, it) }

So if one wants cancellable resource creation, it's as simple as:

val a = acquire()
onRelease { release(a, it) }
onRelease { release(a, it) }

Having cancellable release is a doable change too, but would have to be library-side.
And of course, if your resource involves multiple resources, this approach composes nicely and you can just add the releasers for each sub-resource.

[Youssef Shoaib [MOD]]

The uncancellable stuff looks really interesting! Only comment I have would be to maybe just store the job of the uncancellable caller then withContext(thatJob) inside cancellable. This should have the same effect as your direct continuation manipulation.

[Youssef Shoaib [MOD]]

Regarding the Poll related things, I don't see any issue unifying the Full and non-full APIs in Kotlin. We'd just need to change AcquireStep to be like UncancellableRegion (and perhaps remove that dsl marker restriction)

suspend fun <A> install(
  acquire: suspend AcquireStep.() -> A,
  release: suspend (A, ExitCase) -> Unit,
): A = uncancellable {
  acquire(AcquireStep(this)).also { a -> onRelease { release(a, it) } }
}

and, in fact, we stay source-compatible!
That pattern though would likely be used directly by users instead of calling install.