Skip to content

Closeable with suspendable aClose, and suspendable use extension. #240

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

Closed
wants to merge 1 commit into from

Conversation

programingjd
Copy link

Closeable interface similar to java.io.Closeable, but with a suspendable aClose method.
I used the name aClose to be consistant with aRead and aWrite on channels.
I've added simple tests as well that check that the aClose method is called, whether an exception is thrown or not.

@elizarov
Copy link
Contributor

What kind of abstractions would implement this interface?

@programingjd
Copy link
Author

The obvious use case is for IO.
Right now, if you want non blocking IO, you have two choices.

  1. You use a dedicated thread pool
  2. Use NIO2 async classes

Choice number 2 is better but then you only have fairly low level implementations.
Most of the higher levels abstractions (ObjectInputStream, FileWriter, SSLSocket, ...)
are based on the older blocking apis. Those higher abstractions are among those that
make the most use of the Closeable interface.
What is nice about kotlin suspend methods, is that you can make NIO2 implementations
and usages look like the older NIO code. I believe that was one of the goals of
https://github.com/Kotlin/kotlinx-io.
This makes porting those higher levels abstractions fairly easy (duplicate and add
a bunch of suspend keywords).
Some of them need to have a close method that still does IO (flushing, writing metadata,
sending extra packets, ...) and therefore, the close method should suspend too.

For a more concrete exemple, let's consider a rewrite of an sql database connection that
makes use of AsyncSocketChannel aRead and aWrite (which is what I've been playing with).
PreparedStatement and DatabaseConnection are good candidates for implementing the Closeable
interface, but their close method need to suspend. They need to send the proper packets to
the socket to notify the database to release the associated resources.

@elizarov
Copy link
Contributor

We don't expect that any real-life implementations to use AsyncSocketChannel. This class in JDK has many design problems and should not be really used for serious work.

We are working an alternative asynchronous socket library (code-named "Coroutines IO" or CIO for short) that is directly based on the underlying NIO selectors (and directly on top of Posix in the future). This library extensively uses Job interface to manage lifecycle of various resources (like connection), so when you do job.close() it initiates a connection shutdown and you can do job.join() to wait for it. That is the idea.

@programingjd
Copy link
Author

Alright, I understand that you might not want to use AsyncSocketChannel but something better instead.
However, I don't think this has much to do with the need for a suspendable Closeable interface.
job.join() is a suspendable method, and therefore if you need to call it from the close method, then close needs to be suspendable also.
If we go back to the example of a DatabaseConnection, the close method needs to call suspendable calls on the socket (whether it's with AsyncSocketChannel or cio's job.join())

@elizarov
Copy link
Contributor

We already have an interface for this task. It is called a Job. When we want to represent some resource or background activity that needs to be closed we represent it as an instance of Job. They key feature of the Job is that its close method is a regular function, not suspending. This allows for a clean cancellation (without having to wait), while still providing ability to wait (when needed) via join.

@programingjd
Copy link
Author

I don't think Job is the correct abstraction for this. I think it's too high level. It has a bunch of other methods that don't necessarily make sense for a simple closeable object. The example of the prepared statement is a good example of this. To me, job is more for tasks, and closeable is more for resource-like objects. At least that's what the name suggests, and the methods in the job interface also suggest this.
Maybe Closeable is too low level and you might think it doesn't belong in this library. That's perfectly fine if so. However, a simpler closeable interface with just a suspendable close method would be useful. The Job interface doesn't fit for all the use cases, and the Closeable/use pattern makes as much sense in the suspendable world as they do in the blocking world.

@elizarov
Copy link
Contributor

I'm keeping it open, still looking for more use-case to decide on this abstraction. See we kind of have asynchronous close method on Job -- it is cancelAndJoin extension function. The name is not ideal, but may be it is the asynchronous close we were looking for? But what other primitive are going to implement something asynchronously closeable, without implementing a full-blown Job?

@programingjd
Copy link
Author

I think it's more a matter of semantics.
Closeable is for resources, sockets, files, etc.
Resources don't necessarily do anything. They are just resources. Other classes use them.
Usually with open().use { ... }
Job is more for tasks or actions. Code that actually does something.

In the case that I mentioned, I wouldn't want to have the database connection be a Job. I think it would
be confusing for the user of the api.

@qwwdfsad
Copy link
Collaborator

It's been a half a year since this PR and we still don't have a good use-case in a library, so I'm closing this PR.
I think it will be addressed at some point along with classic Closeable in stdlib

@qwwdfsad qwwdfsad closed this Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants