Use ZManaged for resources in core #249

quelgar · 2020-09-10T10:26:00Z

The goal of this PR is to be a first step in undoing the split between the core and "high-level" modules. The split seemed like a good idea at the time, but in practice the only significant difference between the two modules is the high-level one uses ZManaged, whereas core requires the user to manually release resources using close. This minor difference comes at the cost of maintaining mostly duplicate copies of much of the code.

ZManaged was originally seen as too opinionated as a requirement to use ZIO-NIO. There are situations where lexically scoping resources with use is too limiting. However, ZManaged in ZIO 1.0 offers more flexible resource management via scopes or reservations, so a ZManaged based API shouldn't prevent ZIO-NIO from doing anything that can be done directly with NIO. So this PR makes all resource acquisition in core use ZManaged.

I've also left close as public, which probably seems odd. But close is still useful with ZManaged as an early release mechanism. While ZManaged lets you get an early release effect value, it's often more convenient to just have this effect available on the channel itself. For example, if we want to use the separate early release value with a channel registered in a selector, we have to attach it to the SelectionKey and later get the attachment and cast it to the correct type. But we already have a reference to the channel, so it's a lot simpler to just call close on it directly to release early.

zio.nio.core.channels.SelectorSpec provides an example of what I'm talking about above. I've also added a documentation page describing the resource management options. Hopefully what I've said is accurate 🤞

If we decide to go this way, we can at some point throw out the high-level module and rename zio.nio.core to zio.nio.

svroonland · 2020-09-13T10:12:26Z

docs/essentials/resources.md

+ZManaged.scope.use { scope =>
+
+  val channel: IO[IOException, SocketChannel] = scope(SocketChannel.open).map {
+    case (_ /* early release */, channel) => channel


You could use case (earlyRelease @ _, channel) here to give it a name but avoid an unused param compiler warning.

Good trick, I didn't know that.

svroonland · 2020-09-13T10:16:32Z

docs/essentials/resources.md

+
+### Using `close` for Early Release
+
+In the case of channels, we don't actually need the early release features that `ZManaged` provides, as every channel has a built-in early release in the form of the `close` method. Closing a channel more than once is a perfectly safe thing to do, so you can use `close` to release a channel's resources early. When the `ZManaged` scope of the channel later ends, `close` will be called again, but it will be a no-op.


Is it desirable to expose close to the user in addition to open/close as a ZManaged? With ZManaged you can be sure that the resource is still 'open' as long as the ZManaged is alive, but by exposing close that is no longer the case.

That's true, but the same risk exists if you get the release effect via withEarlyRelease or scope.

svroonland · 2020-09-13T10:25:54Z

nio-core/src/main/scala/zio/nio/core/package.scala

@@ -30,4 +30,11 @@ package object core {
      case _: EOFException => ZIO.none
    }

+  implicit final private[nio] class IOCloseableManagement[-R, +E, +A <: IOCloseable { type Env >: R }](


Any particular reason to use a structured type here instead of trait IOCloseable[Env] ?

When I was experimenting with blocking variants of the channel APIs I found a member type for this was less hassle, and I just copied that idea here. But for this PR, it doesn't actually matter.

I believe using structural types uses reflection, which incurs some runtime overhead. Do you think that would matter for the usage in zio-nio?

Reflection is definitely used to access term-level members, but I think type members are handled entirely at compile time.

Cool, I didn't know that

jdegoes · 2020-10-21T19:01:15Z

@quelgar Conflicts?

quelgar · 2020-10-21T22:12:04Z

@jdegoes good to go

jczuchnowski · 2020-10-25T22:02:10Z

docs/essentials/index.md

@@ -34,7 +34,8 @@ libraryDependencies += "dev.zio" %% "zio-nio" % "1.0.0-RC10"
 ## Main abstractions

 - **[File Channel](files.md)** — For processing files that are available locally. For every operation a new fiber is started to perform the operation.
- - **[Socket Channel](sockets.md)** — Provides an API for remote communication with `InetSocket`s. 
+ - **[Socket Channel](sockets.md)** — Provides anAPI for remote communication with `InetSocket`s. 


typo anAPI

quelgar force-pushed the quelgar/resource-management-core branch from 6f8da72 to bf7a44b Compare September 10, 2020 10:35

svroonland reviewed Sep 13, 2020

View reviewed changes

quelgar force-pushed the quelgar/resource-management-core branch from bf7a44b to 3cbd28d Compare September 23, 2020 10:29

quelgar added 3 commits October 22, 2020 07:08

Switch to using ZManaged for core resources.

2f77409

Add resource management documentation.

a8b9408

Fix extension method on Scala 3.

9945c12

quelgar force-pushed the quelgar/resource-management-core branch from f593378 to 7c9865e Compare October 21, 2020 20:13

quelgar added 2 commits October 22, 2020 07:17

fmt.

ed6ccdf

PR feedback.

25db9f4

quelgar force-pushed the quelgar/resource-management-core branch from 7c9865e to 25db9f4 Compare October 21, 2020 20:18

Merge branch 'master' into quelgar/resource-management-core

2daea56

jczuchnowski reviewed Oct 25, 2020

View reviewed changes

jczuchnowski previously approved these changes Oct 25, 2020

View reviewed changes

Doc typo.

b7eed0c

quelgar dismissed jczuchnowski’s stale review via b7eed0c October 25, 2020 22:43

jczuchnowski approved these changes Oct 28, 2020

View reviewed changes

jczuchnowski merged commit 028a3c8 into master Oct 28, 2020

jczuchnowski deleted the quelgar/resource-management-core branch October 28, 2020 15:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use ZManaged for resources in core #249

Use ZManaged for resources in core #249

quelgar commented Sep 10, 2020

svroonland Sep 13, 2020

quelgar Sep 13, 2020

svroonland Sep 13, 2020

quelgar Sep 13, 2020

svroonland Sep 13, 2020

quelgar Sep 13, 2020

svroonland Oct 22, 2020

quelgar Oct 22, 2020

svroonland Oct 22, 2020

jdegoes commented Oct 21, 2020

quelgar commented Oct 21, 2020

jczuchnowski Oct 25, 2020


		### Using `close` for Early Release

		In the case of channels, we don't actually need the early release features that `ZManaged` provides, as every channel has a built-in early release in the form of the `close` method. Closing a channel more than once is a perfectly safe thing to do, so you can use `close` to release a channel's resources early. When the `ZManaged` scope of the channel later ends, `close` will be called again, but it will be a no-op.

Use ZManaged for resources in core #249

Use ZManaged for resources in core #249

Conversation

quelgar commented Sep 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdegoes commented Oct 21, 2020

quelgar commented Oct 21, 2020

Choose a reason for hiding this comment