Add Bracket type class

[LukaJCB]

Some WIP code I came up with, at some point the future, this might fix #88.

[SystemFw]

Obviously this is just a WIP so mine it's a discussion item and not an actual code review, but this is not the model of bracket I have in mind.
Let's forget interruption and concurrency for a second, but even in the simple sync case, this initial model of bracket can only return IO[Unit], not IO[B].
Consider what happens with your current implementation:

def getFile = bracket(openFile)(file => file.pure[IO])((file,_) => IO(file.close))
def uhOh = getFile.flaMap(f => IO(f.readFirstLine))

by the time we try reading, the file is closed already.
Your type signature is indeed what I would want, but the implementation needs to track currently opened resources across IO operations, and close them appropriately.

P.S. Apologies if this is already perfectly clear to you, but I thought it would be good to specify it anyway for other readers/just in case

[LukaJCB]

Ah you're right, that's a problem indeed, didn't think about that.

[SystemFw]

Another thing to keep in mind is that taking an A in the release action incorrectly assumes that acquire can't fail.

[alexandru]

@SystemFw wait, so let us be clear, is this something that we really want to work?

def getFile = bracket(openFile)(file => file.pure[IO])((file,_) => IO(file.close))
def uhOh = getFile.flaMap(f => IO(f.readFirstLine))

How would that even work? When is f released exactly?

Are you sure you're not thinking of FS2's Stream or Iterant, for which this version of bracket makes sense, due to working with streams?

[LukaJCB]

@SystemFw My assumption on this, is that when acquire fails, it short circuits completely and none of the functions actually get called, no?

SoIO.raiseError(e).bracket(use)(release) wouldn't actually use the resource at all and therefore doesn't need to release it either.

[alexandru]

On an acquire that fails, there's nothing to release since the resource was not allocated. If the user does not want that, then it is trivial to add error handling on the acquire action.

[alexandru]

Just to be clear guys, in my mind bracket is the exact equivalent of this:

val resource = acquire()
try {
  use(resource)
} finally {
  release(resource)
}

If you have something else in mind, then we need to translate it in code like this.

[johnynek]

I don't see how without either linear types or returning F[Unit] we can prevent the kind of error @SystemFw is talking about. We have no way to guarantee A has not been put into the return type of the bracket. It would be nice to have some compiler generated type class like DoesNotReach[B, A] which only has an instance if B has no path to any value of type A. Then we could return F[B] without the worry that it includes the thing that the bracket is protecting.

[SystemFw]

Ah, you're right! Well, good to have that clarified early on :)

[SystemFw]

@alexandru

Are you sure you're not thinking of FS2's Stream or Iterant, for which this version of bracket makes sense, due to working with streams?

Yep, got my wires crossed and was indeed thinking of that, I should probably change topic for today 😄

EDIT: fwiw, I want that bracket will run the finaliser if use is interrupted, not the behaviour described above which you can have with Stream

[alexandru]

A good start, just 2 things:

1. minor mistake: you have an E parameter, so you can't use Throwable anymore
2. we need to be forward thinking and allow cancellation behavior in release — e.g. Monix does not emit an error on cancellation, possibly diverging from other designs and the error type is not enough to distinguish between termination in error and cancellation anyway

I propose this:

trait Bracket[F[_], E] extends MonadError[F, E] {
  def bracket[A, B](acquire: F[A])
    (use: A => F[B])
    (release: (A, Option[Either[E, B]]) => F[Unit]): F[B] 
}

[alexandru]

You'll notice that the current cats.effect.IO will never emit None. Which is actually quite OK, given that this IO can't be cancelled, seems fitting.

[johnynek]

can you add a comment giving an example of why all MonadError does not satify these laws.

[johnynek]

in library code, I prefer to write things the most efficient way, Wonder about all the use of fold and allocation of closures here. What about:

for {
  a <- this
  etb <- use(a).attempt
  _ <- release(a, etb)
  b <- IO.fromEither(etb)
} yield b

Also note, IO.fromEither would probably be better as:

def fromEither[A](e: Either[Throwable, A]): IO[A] =
  e match {
    case Right(a) => pure(a)
    case Left(err) => raiseError(err)
  }

and avoid the allocations of the closures.

[alexandru]

@LukaJCB I'm very happy about this proposal, however it needs to distinguish cancellation, or lets say for now the notion of non-termination:

def bracket[A, B](acquire: F[A])
  (use: A => F[B])
  (release: (A, Option[Either[E, B]]) => F[Unit]): F[B] 

Otherwise it's not very useful for me and I now need it for Iterant.
Any objections to this signature?

[SystemFw]

@alexandru
No objections, but I would be interested if you could expand (when you have time) on

it's enough to say that I believe a cancelled task should be non-terminating

Since you also mention non-termination here :)

[alexandru]

@SystemFw I don't have the energy atm, I'd like to postpone that one, because I'm arguing for async abstractions that have been in Java and .NET and against Haskell and that's not a good place to be in 😀

IMO receiving a Throwable for cancellation is irrelevant in release, since you can't act much on it; maybe there are use-cases that I don't see, hence why I prefer Option because it's simpler.

Either way, the ability to distinguish between cancellation and normal termination is a must that needs to be in the signature. If the signature is now too complicated, we can add helpers as well

[SystemFw]

I don't have the energy atm,

Yeah I sensed that, that's why I added "when you have time" 😄
I'm not coming from a place of arguing against it, just genuinely interested in your opinion ;)

Either way, the ability to distinguish between cancellation and normal termination is a must that needs to be in the signature

Yep 👍

[LukaJCB]

@alexandru I agree, that failure and cancellation should be separate, but didn't implement it yet, because I'm not sure if an option of either is the best encoding of that and when or if we come up with something different I don't have to change all of the code :)

For now I have no better suggestion than what you suggested, however :D

[pchlupacek]

I am perhaps missing something, but how you would get B as result when you cancel it ?

[alexandru]

@pchlupacek you don't, you can either get None, Some(Right(b)) or Some(Left(error)).

[LukaJCB]

@pchlupacek You don't, you'd get a None (in the case @alexandru suggested).

I was thinking just now, maybe it should be three different functions?

def bracket[A, B](acquire: F[A])
  (use: A => F[B])
  (releaseOnError: (A, E) => F[Unit])
  (releaseOnSuccess: (A, B) => F[Unit])
  (releaseOnCancel: A => F[Unit]): F[B]

WDYT? Doesn't have to be curried in this way btw.

[pchlupacek]

I am still a confused here, bracket's result is F[B] if I am reading correctly so how you get that B when you cancel use or acquire ?

[alexandru]

So we have 3 choices:

1. Option, which I think is simple and clear and I prefer it
2. Either[Throwable, Either[Throwable, B]] — which nobody will understand unless they read the docs
3. we introduce our own ADT with 3 states — Success(B), Error(E) and Cancelled(Option[E])

If you want to accommodate all possible variations, then do number 3.
But I'd like to know of a usecase for Cancelled(Some(error)).

[alexandru]

OK, so the problem with different functions is that you don't understand from the signature that only one of those 3 functions will get called, whereas if we do a single function then it's pretty clear due to the input.

[pchlupacek]

Also the signature assumes you may cancel after acquire, but how you would like to resovle asynchronous acquire? That won't be cancellable?

[alexandru]

Also the signature assumes you may cancel after acquire, but how you would like to resovle asynchronous acquire? That won't be cancellable?

That's not a problem because if acquire fails or is cancelled, there's nothing to release and the user can always do error handling for the acquire task specifically. bracket is the equivalent of this:

val resource = acquire()
try {
  use(resource)
} finally {
  release(resource)
}

Obviously you can't put val resource = acquire() inside of try.

[pchlupacek]

yeh, I am on board with that, hence you just release the resource when it is acquired. Agree when it fails, it fails. Having said that how you plan to produce B when you cancel use ?

[alexandru]

@LukaJCB no worries, I'm going to push that in another PR, along with added documentation for the microsite.

[LukaJCB]

Well, it's already done 😄 Thanks again!

[alexandru]

Ah, OK, cool. Will follow up with another PR with some changes anyway 😜

[LukaJCB]

So CI looks good and I think this PR is mergeable now, anyone care for a re-review? @rossabaker @pchlupacek @SystemFw

[alexandru]

Well, I also contributed to it, so I can't approve it 😄

One thing to consider is that this is a blocker for the scheduled 1.0.0-RC.
And I'd really like to do the release tomorrow.

After this gets merged, we'll release a hash version first, to ensure that the laws are passing in Monix as well — laws tend to be written assuming IO's current implementation and miss certain details, especially for the side effects, a problem that I think we got rid of by introducing Pledge in those laws, as an alternative to working with var.

[durban]

-> ExitCase

[LukaJCB]

Thank you, fixed!

[alexandru]

Damn, this needs to implement Serializable at least, maybe Product too 😐

[LukaJCB]

On it!

[rossabaker]

Given that "bracket" is already in extensive and incompatible use in the streaming types (e.g., fs2.Stream. monix.tail.Iterant), I think we might consider a documentation about why this bracket is not that bracket. I don't think it needs to block this merge, but I fully anticipate it being an FAQ.

[rossabaker]

I imagine we want Product here, too?

[alexandru]

@rossabaker you're right, we'll need a FAQ of some sort.

Well, I don't view the bracket on those streaming types to be incompatible, it's the same spirit, but the signature is a little different. We'll need to mention it and explain the difference though.

Can it happen after the merge?

[rossabaker]

Yes, I don't want the FAQ to delay the merge.

I tried to think of a nice alternate name for either this bracket or Stream/Iterant style bracket, and failed. They both seem pretty well established under this name.