Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource[F, A]#memoize releases right after first use #3079

Closed
seigert opened this issue Jul 7, 2022 · 5 comments · Fixed by #3080
Closed

Resource[F, A]#memoize releases right after first use #3079

seigert opened this issue Jul 7, 2022 · 5 comments · Fixed by #3080

Comments

@seigert
Copy link
Contributor

seigert commented Jul 7, 2022

Scastie link: https://scastie.scala-lang.org/wxfOvT64RDSGkjRfeAqWgA

Memoized resource allocates just before the first use (as expected), but releases right after it neither holding resource until 'memo release' nor allocating anew before second usage:

scala> val memo = Resource.make(IO.println("Allocate"))(_ => IO.println("Release")).memoize

val memo:
  cats.effect.kernel.Resource[cats.effect.IO,
    cats.effect.kernel.Resource[cats.effect.IO, Unit]
  ] = Bind(Bind(Eval(IO(...)),cats.effect.kernel.Resource$$Lambda$11226/0x000000080229f008@5e3f1864),cats.Monad$$Lambda$11228/0x000000080229f9c8@1ce88df5)

scala> memo.use { r =>
     |   for {
     |     _ <- IO.println("First:")
     |     _ <- r.use(_ => IO.println("First done"))
     |     _ <- IO.println("Second:")
     |     _ <- r.use(_ => IO.println("Second done"))
     |   } yield ()
     | }.unsafeRunSync()
First:
Allocate
First done
Release
Second:
Second done

Cats-Effect 3.3.13, Scala 2.13.8

@djspiewak
Copy link
Member

Okay @SystemFw, you can say I told you so. 😛

This is a known issue. Or rather, it's actually defined this way in general. The problem is that this definition of memoize follows directly from the definition of scoping in Resource itself. It isn't really "solvable" per se.

The long and short of the problem is the use of use. There isn't a good way for that function to work in these kinds of contexts, and one of the things it does in this case is close the hidden scope. If you avoid calling use twice on the memorized resource and instead rely on flatMap with a single use at the end, it will work the way you expect.

armanbilge added a commit to armanbilge/cats-effect that referenced this issue Jul 7, 2022
@armanbilge
Copy link
Member

It isn't really "solvable" per se.

Yeah, why is that? Seems straightforward enough to override the implementation to do the right thing, I tried this in #3080.

@armanbilge
Copy link
Member

Ah, this issue looks like a dupe of #1620.

@seigert
Copy link
Contributor Author

seigert commented Jul 7, 2022

It isn't really "solvable" per se.

The long and short of the problem is the use of use. There isn't a good way for that function to work in these kinds of contexts, and one of the things it does in this case is close the hidden scope.

@djspiewak, I'm not sure I follow. :( Resource#use scaladoc contains requirement "The resource is released as soon as the resulting F[B] is completed, whether normally or as a raised error", but it could be interpreted as 'the actual holder of resource is the memo, and internal resource is just a shallow copy of it'. This way use of memo need to release actual resource, but 'inner' resources do not.

As for implementation, semaphore + ref counter could solve #3080 issue of allocation on release.

@seigert
Copy link
Contributor Author

seigert commented Jul 7, 2022

I mean, for every val r: Resource[F, A] I can do val memoR = r.map(a => Resource.eval(R.pure(a))) and it seems to me it would be equivalent to memoize in everything but lazy allocation (edit: the same as fa.map(a => F.pure(a)) is equivalent of fa.memoize in everything but lazy evaluation).

And if we have def foo(r: Resource[F, A): F[B] = ??? we actually have no means to guarantee that A we be allocated just for the foo or disposed in it, we just assume so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants