NOT_INTERRUPTIBLE OpenOption and how that should fit with InterruptibleChanne

[Bruno Ploumhans]

The different options are a good way to see it. I think that #1 is too
breaking. Among the others,
- #2 does not change default behavior so it is the least breaking,
- #3 does change default behavior, but allows opting back into old behavior.
- #4 does change default behavior, with no option to revert to the old behavior.

#3 is weird in that it changes the default behavior, yet acknowledges
that it's a breaking change that might be undesirable.

Beyond the compatibility impact, implementing #4 also removes the
ability to interrupt potentially blocking network file operations,
which might already make it a no-go? See [1] for an example of a
FileChannel that wraps a blocking network operation; I am not sure how
relevant it is.

> #2 [...] is also the least satisfactory option too.

My $0.02 as a low-experience NIO user: I can understand why a
FileChannel would be interruptible, and as long as I can opt out of
that behavior I am happy. One would just have to learn the hard way
that FileChannels are interruptible by default...

Best,
Bruno

[1]: https://github.com/apache/mina-sshd/blob/master/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpRemotePathChannel.java

Le dim. 5 janv. 2025 à 10:53, Alan Bateman <alan.bateman at oracle.com> a écrit :
>
> On 26/12/2024 10:15, Bruno Ploumhans wrote:
>
> Hi,
>
> I opened a PR that adds StandardOpenOptions.NOT_INTERRUPTIBLE to
> expose and supersede FileChannelImpl#setUninterruptible. However, this
> needs more discussion because FileChannel implements
> InterruptibleChannel.
>
> Here are some thoughts to kick-start the discussion:
> - The documentation of InterruptibleChannel does not currently account
> for implementations that might not be interruptible. Same for the
> documentation of Thread.interrupt(). That needs changing.
> - Alan suggested in previous discussion (2018) a new `boolean
> isInterruptible()` method.
> - I wonder if the ability to pass a `boolean uninterruptible` flag
> should be exposed in AbstractUninterruptibleChannel, or if that should
> be kept an implementation detail of FileChannelImpl.
>
> In any case, I am happy to experiment with various solutions.
>
> This is a topic that I've looked several times including explorations and prototypes of four options:
>
> 1. Changing FileChannel to not extend AbstractInterruptibleChannel
> 2. An OpenOption to opt out of interrupt closing the channel
> 3. An OpenOption to opt in to interrupt closing the channel
> 4. Specifying that interrupt does not close the channel but specified to allow for compatibility with long standing behavior
>
> There are pros and cons to all.
>
> For the options explored then most of the change is specification with the starting point for all was to re-specify async close of FileChannel to align with long standing behavior. If you look at how async close of a FileChannel works on Windows or with virtual threads then you'll see what I mean. There is also the issue that the lock methods specify that FileLockInterruptionException is thrown without closing the channel but the implementation differs.
>
> My view on this has changed from when I first looked at it a few years ago. Right now, I think #4 may be the best option. The spec changes that I have from that option are:
>
> - Changes to FileChannel to specify that it deviates from InterruptibleChannel to allow for close to wait for all outstanding I/O operations to end. This allows for long standing behavior. As part of this, FileChannel overrides the specification of implCloseChannel.
>
> - Changes to FileChannel to specify that it deviates from InterruptibleChannel so that interrupt does not close the channel but allows for an implementation specific means for interrupt to close the channel.
>
> - Changes to the specification of Thread.interrupt, ClosedByInterruptException, all methods in FileChannel that throw ClosedByInterruptException, FileLockInterruptionException and the FileChannel lock methods.
>
> The main thing with this option is identifying any common usages where ClosedByInterruptException may be depended on, e.g. trying to shutdown something when threads are blocked indefinitely in file I/O operations. Such usages are likely file access over a network or maybe transfer to/from a channel backed by a network connection. Any help to identify such usages would be helpful to assess the compatibility impact of the options that change default behavior.
>
> Your exploration is #2. The spec changes that I did when prototyping this option including changes to all the above spec areas, and also everywhere that opens a file, e.g. AsynchronousFileChannel. It is clearly the least risky from a compatibility perspective as it doesn't change default behavior but also the least satisfactory option too.
>
> -Alan
>