Cats-effect: Bracket's acquire should not be uncancelable by law

[alexandru]

We have this law in BracketLaws:

def acquireAndReleaseAreUncancelable[A, B](fa: F[A], use: A => F[B], release: A => F[Unit]) =
  F.bracket(F.uncancelable(fa))(use)(a => F.uncancelable(release(a))) <-> F.bracket(fa)(use)(release)

It's a good thing that this is essentially a no-op when executed, because we shouldn't have it. This is then explained more in ConcurrentLaws via the interruption powers of Concurrent:

def acquireIsNotCancelable[A](a1: A, a2: A) = {
    val lh =
      for {
        mVar  <- MVar[F].of(a1)
        latch <- Deferred.uncancelable[F, Unit]
        task   = F.bracket(latch.complete(()) *> mVar.put(a2))(_ => F.never[A])(_ => F.unit)
        fiber <- F.start(task)
        _     <- latch.get
        _     <- F.start(fiber.cancel)
        _     <- contextShift.shift
        _     <- mVar.take
        out   <- mVar.take
      } yield out

    lh <-> F.pure(a2)
}

The problem: acquire being uninterruptible is an implementation detail that doesn't work for _streams_, because for streams we need the following equivalence:

acquire.bracketCase(use)(release) <-> acquire.flatMap { a => use(a).guaranteeCase(release(a, _)) }

And this is yet another case where the auto-cancelable model rears its ugly head. Without having the possibility to cancel flatMap chains (e.g. the initial behavior in 0.10, then proposed as the "Continual" model, see #262), we would have this equivalence for all F[_].

Right now, if I were to implement this in a way that's "lawful" for Observable or Iterant, we would be speaking of this:

acquire.uncancelable.flatMap { a => use(a).guaranteeCase(release(a, _)) }

And of course I cannot do this for Observable or Iterant. This makes absolutely no sense in case of streams. To make matters worse, we went ahead and described this in Bracket itself:

def uncancelable[A](fa: F[A]): F[A] =
  bracket(fa)(pure)(_ => unit)

As it is, implementation details have leaked in the type class design that make the type class hierarchy incompatible with streams. Given that Bracket is essentially at the level of MonadError, that's regrettable and it needs to be fixed by deprecating those laws and maybe the default implementation too.

I'm currently attempting a fix.

[alexandru]

See PR: https://github.com/typelevel/cats-effect/pull/383

[jdegoes]

If acquire can be cancelled / interrupted, then resources can be leaked quite trivially:

(openFile(path) <* IO.cancelBoundary <* doOtherStuff <* IO.cancelBoundary).bracket(closeFile(_))(processData(_))

In general, the acquisition won't even be an atomic sync but a composition of many smaller IO operations.

FS2 doesn't implement Bracket for stream. I think perhaps they're too different to be unified; or at least, that they shouldn't be unified at the cost of resource safety.

[RaasAhsan]

Due to the implementation of IOBracket, isn't it impossible to cancel an acquire, its run loop being started with an uncancelable IOConnection?

Is there any discussion on why the auto-cancelable model was used? It does seem more flexible to me to be able to specify your own cancellation boundaries.

[mpilquist]

I think the current version is correct -- acquire should be uncancelable for all instances of Bracket. The example posted by @jdegoes is a good example as to why.

[SystemFw]

It does seem more flexible to me to be able to specify your own cancellation boundaries.

How so? The argument _against_ the auto-cancelable model is that it is _too_ flexible (I don't think it's a winning argument, but it does hold value).

Also, you're not always in control of the code you want to cancel, so having to set cancelation boundaries manually is somewhat anti-modular (the author of the code needs to anticipate where the users might want to cancel the code).

---

Generally speaking, I'm not convinced by the argument that "we wouldn't have this problem if it wasn't for the auto-cancelable model", it sounds similar to having:

  for {
    st <- Ref[IO].of(0)
    v <- st.get
    _ <- st.set(v + 1)
 }  yield ()

and saying that this isn't equivalent to State, because the concurrent model rears its ugly head.

"we wouldn't have this race if it wasn't for concurrency" is true, but the usual solution would be to fix the race, not to prohibit concurrency (which is what Continual does, by this analogy).

---

Now, for the Stream thing. In my _personal_ opinion, bracket for Stream and IO (pick any streaming/F types here) is too different, and that's due to a fundamental distinction between the two types.
We chose to not have fs2.Stream implement bracket, but I know Alex really cares about streaming types being compatible with effect classes. I respect that, and I'm not opposed to it in principle, but I'm afraid that we might end up relaxing Bracket so much that it doesn't guarantee safety for F types, which is its primary goal.

[alexandru]

Sorry guys, false alarm, apparently I'm wrong:

• Iterant was already correct since bracket is encoded in its ADT and it takes IO.bracket to interpret it
• I could make Observable behave well, I just needed to sleep on it 🙂

So apparently I've been very superficial about this proposal.
Thank you all for the input.

---

@SystemFw

Now, for the Stream thing. In my personal opinion, bracket for Stream and IO (pick any streaming/F types here) is too different, and that's due to a fundamental distinction between the two types.

I disagree, it's the same.

The difference between IO-like types and streams is basically flatMap and we have the same flatMap for both IO and streams by having laws for Monad that don't assume IO or equivalent. This is why I care very much about supporting streams in these type classes, otherwise we risk introducing assumptions of behavior that aren't explicit.

[SystemFw]

I disagree, it's the same.

This is the signature for fs2.Stream.bracket:

 def bracket[F[_], R](acquire: F[R])(release: R => F[Unit]): Stream[F, R]

Which literally makes no sense for IO.

The difference between IO-like types and streams is basically flatMap and we have the same flatMap for both IO and streams

I can't parse this statement, it seems to be self-contradictory.

Anyway, I don't disagree with the attempt of making the hierarchy support streaming types per se, as long as we don't end up weakening support for F types to a critical extent. I agree that thinking about streaming support is good for checking our assumptions about behaviour, at the very least.

[alexandru]

So I made the mistake of thinking that Iterant is a solved deal. Fixed Observable which now works, but not Iterant. At this point I don't think I can, but I'm not absolutely sure.

I'm neck deep in technical details, sorry for the confusion. Will not reopen the issue or the PR with no consensus, which seems to be the case.

---

@SystemFw

Just to be on the same page, I have this signature in Iterant as well:

def resource[F[_], R](acquire: F[R])(release: R => F[Unit]): Iterant[F, R]

def resourceCase[F[_], R](acquire: F[R])(release: (R, ExitCase[Throwable]) => F[Unit]): Iterant[F, R]

But what we are talking about is this:

def bracket[F[_], R](acquire: Stream[F, R])(release: R => F[Unit]): Stream[F, R]

If this one exists, you can express the former in terms of this one, being strictly more powerful.

Of course, the former is the more useful one to have for streams. But unfortunately we made the choice of placing Bracket at the top of the hierarchy, so now these laws are imposed on the whole type class hierarchy of Cats-Effect.

In other words, versus what we had in 0.10, we introduced extra restrictions on what types can implement Sync. I cannot express how stressful this is for a library author to find that, by migrating from 0.10 to 1.0.0, Sync is no longer possible.

And going back to our roots, Sync is MonadError with a stack safe flatMap and the ability to suspend side effects. That a Sync data type needs to implement a bracketCase with an "uncancelable" acquire in bracket is at this point a big problem.

"we wouldn't have this race if it wasn't for concurrency" is true, but the usual solution would be to fix the race, not to prohibit concurrency (which is what Continual does, by this analogy).

In order to fix races, the best kind of fix is to prohibit concurrency.

We had to rebuild the entire implementation of Iterant, therefore when I'm saying that I want my continual model to be a possibility, that's because I suffered by literally changing thousands of lines of code.

And now I'm finding that Sync might not be possible for Iterant ... and it's in essence the exact same reason. And I'll probably just give up on the idea, because it's too painful to be the only one that wants this.

[SystemFw]

Right, so actually what I wanted to put the stress on in that particular comment wasn't whether acquire has F, as in

def resource[F[_], R](acquire: F[R])(release: R => F[Unit]): Iterant[F, R]

or Iterant, as in

def bracket[F[_], R](acquire: Iterant[F, R])(release: R => F[Unit]): Iterant[F, R]

but rather, that when you compare either of those with F.bracket, there's no use, which it _has_ to be there in F.bracket. So I'm not entirely clear how either of those signatures play a role here, or indeed how F.bracket and Stream.bracket are the same, given the fundamental distinction about use vs resource lifetime that extends over flatMap.

---

Now, I do agree that placing Bracket at the top of the hierarchy has hindered your ability to provide a Sync for Iterant (and I'm sorry for the pain and extra development this has caused you).
A few thoughts:

• Having a Sync without any form of resource safety guarantee sounds dangerous. Even if we forget about interruption and streams, just MonadError doesn't even work for transformers, so without Bracket you are very limited in the instances you can provide.
• Due to the differences in Stream and F bracket, it's looking hard (or impossible) to provide a single abstraction for resource safety that works for both.

And this leads me to what I think is our base disagreement on this: I think your approach is that extending effect type classes to streaming types should just work, provided the hierarchy is designed correctly. Otoh, I think that Stream and IO (pick any other two) are two fundamentally different abstractions, and that trying to extend effect classes to streams is a massive step up in how ambitious the type classes are meant to be.

In fact, I've barely even seen a typeclass based abstraction for streaming types only, let alone one that abstracts over them _and_ F types.

This isn't to say that trying to do that is useless or wrong, but rather that it _might_ be the case that expecting our effect type classes to work for streams as well might be setting the bar too high.

[alexandru]

@SystemFw @mpilquist Just another datapoint — in Semaphore we have this:

def withPermit[A](t: F[A]): F[A] =
  F.bracket(acquire)(_ => t)(_ => release)

acquire can block forever, therefore the result isn't cancelable and is a memory leak exactly for the kind of use case for which we want to avoid memory leaks — consider that acquiring a lock is exactly the kind of situation for which you want to implement a timeout and for withPermit this isn't possible without a memory leak.

But wait, there's more — because of the auto-cancelable model of evaluation, bracket is the only way to acquire and release such locks safely, so there's absolutely no way to timeout on Semaphore's acquire.

Looking at the acquire implementation, I'm noticing that it isn't cancelable and this is probably the reason why. The Semaphore that I'm pushing in Monix however does have a cancelable acquire.

We can have the same discussion btw with MVar's take and put. Or for any such pair of operations — e.g. if we had a Queue we could be discussing about offer and poll.

bracket's acquire should not be uncancelable and coupled with the auto-cancelable model makes the 1.0.0 upgrade to be strictly a downgrade from 0.10 in terms of safety. But it's too late for any changes to 1.0.0 of course.

[mpilquist]

Looking at the acquire implementation, I'm noticing that it isn't cancelable and this is probably the reason why. The Semaphore that I'm pushing in Monix however does have a cancelable acquire.

cats.effect.Semaphore#acquireN is cancellable if constructed with a Concurrent[F] -- take a look at the implementation of awaitGate in the concurrent impl.

[jdegoes]

@alexandru

acquire can block forever, therefore the result isn't cancelable and is a memory leak exactly for the kind of use case for which we want to avoid memory leaks — consider that acquiring a lock is exactly the kind of situation for which you want to implement a timeout and for withPermit this isn't possible without a memory leak.

Acquire cannot be cancelable / interruptible. To do so is resource unsafe. The correct solution is to disallow asynchronous acquire actions whose suspensions consume unreleasable memory (i.e. those which could "block" forever in a way that consumes memory and cannot be garbage collected).

There's no type safe way to do this. You could restrict acquire to SyncIO, but that's not exactly the right restriction—because some async computations (e.g. IO.sleep(1.ms)) will either not suspend forever or will not consume unreleasable memory (because the object holding the suspension can be garbage collected, if unreferenced).

cats.effect.Semaphore#acquireN is cancellable if constructed with a Concurrent[F] -- take a look at the implementation of awaitGate in the concurrent impl.

Unfortunately, it does not matter if it's cancellable because the acquire operation of bracket will not be cancelled, so as soon as it occurs in bracket or withPermit, the cancellable nature of the acquisition becomes irrelevant.

Bottom Line: Acquire cannot be cancelable, it could at best be forced to be synchronous, with some loss in expressive power (e.g. it would disallow withPermit). Ultimately Semaphore is a low-level, leaky construct that's very easy to misuse and should be used with the greatest of care in infrastructure libraries like FS2, etc, not generally directly by end users.

[alexandru]

@jdegoes

Acquire cannot be cancelable / interruptible. To do so is resource unsafe.

That's factually false, an acquire that cannot be canceled in the context of bracket is what's resource unsafe.

Again I'm presenting the use-case of Semaphore — currently there's absolutely no way to use acquire and release in a way that doesn't leak memory.

Unfortunately, it does not matter if it's cancellable because the acquire operation of bracket will not be cancelled, so as soon as it occurs in bracket or withPermit, the cancellable nature of the acquisition becomes irrelevant.

Exactly, which is what I'm complaining about.

The bracket operation is broken and unsafe by law and Semaphore's withPermit is definite proof.

[alexandru]

@mpilquist btw, that acquire is cancelable is irrelevant, which is what John is saying — this is because it can only be used via bracket and there's no way to observe cancelation in a safe way that doesn't involve bracket.

For example if people think of doing this:

bracket(acquire.timeout(4.seconds))(_ => fa)(_ => release)

This doesn't work because you cannot know if acquire managed to acquire the resource or not when cancelling it via that timeout. So there's no way to use acquire in a cancelable way, without a corresponding release (which is always the case in 100% of the time) and do so safely, because concurrency.

[jdegoes]

That's factually false, an acquire that cannot be canceled in the context of bracket is what's resource unsafe.

I've previously explained in detail with examples why an acquire that can be cancelled / interrupted is resource unsafe. I do not wish to explain it again. See previous conversations on the topic, or let's agree to disagree.

Again I'm presenting the use-case of Semaphore — currently there's absolutely no way to use acquire and release in a way that doesn't leak memory.

You're presenting a leaky, dangerous abstraction (Semaphore), which may be necessary (or may not be) but should only used with extreme caution by those who know what they're doing.

Any acquire action that suspends indefinitely and consumes unreleasable memory will suffer from the same issues as Semaphore. That's not a reason to destroy the resource safety of bracket.

[alexandru]

Semaphore is a fundamental abstraction in all of computer science. This is not mere disagreement.

bracket, as currently defined, is unsafe and unfit for primitives.

If its acquire would be interruptible, you could have the current behavior by simply making acquire to be atomic via uncancelable. But there's no way, absolutely no way to go in the other direction.

[alexandru]

As for your previous example:

(openFile(path) <* IO.cancelBoundary <* doOtherStuff)
  .uncancelable
  .bracket(processData(_))(closeFile(_))

There, fixed.

[SystemFw]

I think this is relevant

http://hackage.haskell.org/package/base-4.12.0.0/docs/Control-Exception.html#g:13

[jdegoes]

Semaphore is a fundamental abstraction in all of computer science. This is not mere disagreement.

No. It's a fundamental abstraction in (thoroughly) broken models of concurrency, which have been the source of unending pain and frustration for developers who have used them.

Semaphore requires extreme attention to care and detail in order to guarantee that acquires are properly paired with releases in the presence of errors, cancellation, interruption, defects in the code, and so forth.

bracket, as currently defined, is unsafe and unfit for primitives.

Only for primitives which are already dangerous and unsafe.

There, fixed.

Poor design IMO. In the vast majority of cases—we are talking business developers who are using libraries like Monix, FS2, Doobie, Cats Effect, ZIO, etc—failure to make acquire uncancelable is a bug in the code that will _definitely_ lead to leaking resources. Not _probably_, but _definitely_.

WORSE STILL:*

Making acquire interruptible "solves" one problem at the cost of introducing another. Mainly because in the implementation of acquire for Semaphore, the state change of the underlying Semaphore 's Ref occurs chronologically prior to the return of the acquire action. If interruption is performed right after the state is changed inside the Ref but right before the acquire action returns, then you end up leaking resources (they will never be released; the finalizer will never be run because acquire never returned).

Even if you could make the modify of Ref become the final F[_] returned from acquire (i.e. rewrite the acquire implementation of Semaphore), a developer who introduces an innocuous subsequent bind or even refactors code slightly in a way allowed for by monad laws (e.g. <* IO.point(1)) will now break the correctness of the code.

The correctness of code cannot be finicky to refactorings allowed for by monad laws. To do so would introduce catastrophic consequences for reasoning. Rather than piling hacks upon hacks that only mask problems and create other problems that are even worse than the original problem, the best course of action is to accept that acquire must be uninterruptible for bracket to have any meaning at all.

Further, if you want a version of bracket that allows for interruptions / cancellations on acquire, _you can implement it already on top of ensuring_. You will still run into the fundamental problem that you cannot know where in acquire the action is interrupted (whether before or after release), making correctness finicky. The best solution to this problem would be to introduce a type that represents an interruptible single-step suspension, which cannot be composed sequentially with other actions, and ensure this interruptible bracket takes such an action for its acquire. But that's a separate solution with a different API for a relatively fringe use case and should not affect the design of bracket. IMO.

[alexandru]

Making acquire interruptible "solves" one problem at the cost of introducing another. Mainly because in the implementation of acquire for Semaphore, the state change of the underlying Semaphore 's Ref occurs chronologically prior to the return of the acquire action. If interruption is performed right after the state is changed inside the Ref but right before the acquire action returns, then you end up leaking resources (they will never be released; the finalizer will never be run because acquire never returned).

I cannot speak for Cats-Effect's implementation, however I can speak for the implementation in Monix, which is guaranteed to have the correct behavior.

Further, if you want a version of bracket that allows for interruptions / cancellations on acquire, you can implement it already on top of ensuring

No, you cannot and I dare you to provide the code for it.

[alexandru]

Just so we are on the same page, here's the Monix implementation:

1. interface
2. actual implementation

This implementation has guaranteed, safe cancellation semantics. There's no way to break it.

Also, if we could express a bracket with interruptible acquire via guarantee, then it would be a non-issue. Unfortunately I don't think we can, hence I would be glad to be proven wrong.

[jdegoes]

No, you cannot and I dare you to provide the code for it.

In ZIO, bracket is implemented in terms of ensuring.

It's easy to provide an interruptible one:

def interruptibleBracket[E, A, B](
  acquire: IO[E, A])(
  release: A => IO[Nothing, Unit])(
  use: A => IO[E, B]): IO[E, B] = 
  for {
    ref <- Ref[Option[A]](None)
    b   <- acquire.flatMap(a => ref.set(a) *> use(a)).ensuring(ref.get.flatMap(_.fold(IO.unit)(release)))
  } yield b

The problem is it's not safe. Even if you disable "interruptible binds" between acquire and ref.set(a), you still don't know where inside acquire the resource is actually acquired.

In the case of a Semaphore, the implementation of acquire will do something like ref.modify(...). If the modification of the state is not the _last_ action in acquire, if you even do something like ref.modify(...).void (or, more commonly, F.flatten(ref.modify(...))), then suddenly the code is broken.

This is an insane way to program, which is why it should not be encouraged or supported. Semaphore will always be dangerous and must only be used by experts in special circumstances where they know exactly what they are doing.

[alexandru]

@jdegoes that's the point — the implementation you provided is not safe, unless it's provided by the platform, by IO's run-loop, which can make it safe.

Which is what I'm campaigning for — we need a interruptibleBracket to be provided as a primitive, otherwise we cannot implement its semantics.

[alexandru]

And if we have interruptibleBracket, then normal bracket is derived from interruptibleBracket + uncancelable, right?

[jdegoes]

@alexandru You keep missing my point: which is that even if you provide interruptibleBracket, it's still exactly as unsafe as the version above, because you can add <* IO.unit to the end of your Ref modification and now it leaks permits.

The only way to do this safely is providing a version of bracket whose acquire is a single-step interruptible suspension. There does not yet exist any such thing in the ecosystem.

[alexandru]

So you mean the auto-interruptible flatMap you introduced are becoming an argument for bracket, and bracket becomes an argument for auto-interruptible flatMap operations, right?

That's a circular argument.

[jdegoes]

@alexandru Not really related to async exceptions. Add <* IO.cancelBoundary <* ... to the end of the acquire, and you're in the same boat. Or just add <* doOtherThing for some doOtherThing that's not local and that you didn't write, which could potentially introduce a cancelation boundary. Local reasoning means you shouldn't have to know what "comes after" an atomic state modification like Ref.modify in order to determine if your code is correct.

If you're committed to making acquire interruptible, you absolutely must eliminate interruption and cancellation. In which case you don't need to make acquire interruptible anymore, since you eliminated interruption and cancellation. 😆

[alexandru]

@jdegoes your argument doesn't hold because even in the current model you can always create leaks via:

bracket(acquire <* IO.raiseError(e)) ...

There, now the current bracket leaks and there's nothing you can do about it.

I agree that the current bracket is less error prone for say 80% of use cases. However let's assume for the sake of argument that there are people like me that know what they are doing.

For example, I have an AsyncSemaphore described in monix-execution (the low level stuff) that works with CancelableFuture and its implementation of withPermit is safe.

When a Future-based implementation is safer than the IO/Task equivalent, that's when you know things went terribly wrong 😉

[jdegoes]

your argument doesn't hold because even in the current model you can always create leaks via:

In ZIO you know if the subsequent computation can fail by examining the compile-time error type.

I agree that the current bracket is less error prone for say 80% of use cases. However let's assume for the sake of argument that there are people like me that know what they are doing.

As a poor man's solution (see above for what I believe the real solution to be), you can always implement your own cancelation layer on your own version of bracket. i.e. taking a cancelable action, and have it start up an uncancelable action in a separate fiber, which listens to an interrupt signal, and takes appropriate action. Whatever semantics you are imagining for an interruptible bracket, you could provide them in this fashion.

But your proposed solution to merely make acquire interruptible / cancelable doesn't solve the real problem; you would have to at minimum make acquire cancelable but not interruptible (which means you would need separate models for interruption and cancelation), and then you would still suffer from the problem that if a user accidentally sequences a cancel boundary inside an acquisition but before they return, then they will leak resources.

When a Future-based implementation is safer than the IO/Task equivalent, that's when you know things went terribly wrong

Nope, because Future does not support pervasive, application-wide timeouts, early termination for parallel and racing computations, and so forth. Ordinary imperative synchronous programming with Java's Semaphore is easier and has stronger guarantees than using the cats-effect Semaphore. That doesn't make it better.

[alexandru]

Nope, because Future does not support pervasive, application-wide timeouts, early termination for parallel and racing computations, and so forth

You haven't seen my Future John, of course it supports timeouts and cancellation:

https://github.com/monix/monix/blob/master/monix-execution/shared/src/main/scala/monix/execution/CancelableFuture.scala

[jdegoes]

You haven't seen my Future John, of course it supports timeouts and cancellation:

Cancellation or interruption? They are not the same thing.

[alexandru]

For our purposes here they are the same thing and trying to draw a distinction is misleading:

import monix.execution._

def race[A](f1: CancelableFuture[A], f2: CancelableFuture[A]): CancelableFuture[A] = {
  val p = Promise[A]()
  f1.onComplete { r => f2.cancel(); p.tryComplete(r) }
  f2.onComplete { r => f1.cancel(); p.tryComplete(r) }
  CancelableFuture(p.future, Cancelable.collection(f1, f2))
}

def delayResult[A](r: => Try[A], d: FiniteDuration)
  (implicit s: Scheduler): CancelableFuture[A] = {

  val p = Promise[A]()
  val c = s.scheduleOnce(d)(p.complete(r))
  CancelableFuture(p.future, c)
}

def timeout[A](f: Future[A], d: FiniteDuration)
  (implicit s: Scheduler): CancelableFuture[A] = {

  race(f, delayResult(Failure(new TimeoutException), d))
}

/// this works ;-)

val semaphore = AsyncSemaphore(provisioned = 10)

timeout(semaphore.withPermit(() => task), 5.seconds)

[jdegoes]

For our purposes here they are the same thing and trying to draw a distinction is misleading:

They are not remotely the same thing.

Interruption means, for example, that I can create a large program only from the methods of Monad, gradually composing smaller pieces until I have a massive program which does tremendous computation, and then instantaneously interrupt the execution of this program without any additional code; it's the ability to stop the runtime interpretation of the monad, not at specified points, but anywhere, with a guarantee that all finalizers will be run.

Cancellation is more similar to boolean-flag checking at defined points.

[alexandru]

Well, yes, we can talk about my "continual" model proposal, because I do want cases in which auto-cancelation (aka interruption) to not be active, precisely because having fixed, user defined points for cancelation is more predictable and the auto-cancelable model doesn't buy you much.

Your argument that you're building just based on Monad, I don't agree with it — it's not just Monad, it's Monad + bracket. If it were just Monad, then my original Iterant encoding would have been perfectly valid, but it's not just Monad, it's Monad + an operation that cannot be implemented by all monads.

But that's another discussion entirely — at this moment I'd like to focus on the job at hand, which is having the possibility of defining a safe Semaphore.withPermit.

Having the ability to use a Semaphore and to safely timeout a withPermit is what users expect and we need to find ways to make it happen.

[alexandru]

... or I can point people to AsyncSemaphore 🤭

[jdegoes]

But that's another discussion entirely — at this moment I'd like to focus on the job at hand, which is having the possibility of defining a safe Semaphore.withPermit.

I've suggested several ways. It just can't be done by making acquire interruptible. 😄

Your argument that you're building just based on Monad, I don't agree with it

Building just on monad (without finalizers) results in a program that's instantaneously interruptible. This is the core distinction between interruption and cancellation. Interruption is pervasive and comes for free (even just using monad operations), cancellation is explicit and occurs at user-defined boundaries. Only one of these models is composable.

... or I can point people to AsyncSemaphore

Indeed. One can argue they belong in 3rd party libraries.

[mpilquist]

Semaphore#withPermit resource leak fixed in #403.

@jdegoes ZIO's semaphore might need a similar fix as it's bracketing on acquire currently: https://github.com/scalaz/scalaz-zio/blob/master/core/shared/src/main/scala/scalaz/zio/Semaphore.scala#L25

[jdegoes]

@mpilquist Oh wow, that was simpler than I thought. Nicely done! 🎉

Rather than interruption on acquire, just let the acquire finish and immediately release! 😆

I believe the new implementation of Semaphore in ZIO does not have this problem because it uses Promise.bracket but I'll check to make sure.