RFR: 8276779: (ch) InputStream returned by Channels.newInputStream should have fast path for SelectableChannels [v14]

[Alan Bateman]

On Sun, 21 Nov 2021 09:54:11 GMT, Markus KARG <duke at openjdk.java.net> wrote:

>> test/jdk/java/nio/channels/Channels/TransferTo.java line 198:
>> 
>>> 196: 
>>> 197:         // IllegalBlockingMode must be thrown when trying to perform a transfer
>>> 198:         assertThrows(IllegalBlockingModeException.class, () -> is2.transferTo(os2));
>> 
>> Thanks for adding this test. It looks like it leaks 5 file descriptors (the FileChannel, and the source/sink of two pipes). Can you update these to close the channels? The test current runs in othervm mode but if it is changed back to agentvm mode then leaving channels open could impact tests that are run later in the same VM.
>
> I sketched a possibly solution in https://github.com/openjdk/jdk/pull/5179/commits/26ed670be318669edf1ef9a7101c1136273f2b82, but looking at the PipeImpl internals I doubt that *all* file descriptors actually get closed. To me it feels like this being actually a design flaw of PipeImpl: If one side of a pipe was never actually claimed then it should not keep a descriptor open. So IMHO the final solution would be to file a PR for PipeImpl which either defers opening of the file descriptors until they are claimed (which I doubt would be easily possible), or which cleans up both sides if as soon as one side is closed and the other side never got claimed (which is my personal preference).

> I sketched a possibly solution in [26ed670](https://github.com/openjdk/jdk/commit/26ed670be318669edf1ef9a7101c1136273f2b82), but looking at the PipeImpl internals I doubt that _all_ file descriptors actually get closed. To me it feels like this being actually a design flaw of PipeImpl: 

A Pipe creates a pair of channels. If you change to something the following then it will fix the issue:


Pipe pipe = Pipe.open();
try {
    :
} finally {
    pipe.source().close();
    pipe.sink().close();
}

-------------

PR: https://git.openjdk.java.net/jdk/pull/5179