Re: Behaviour of SocketChannelImpl.close() in Java11 (ea+12)

[Bernd Eckenfels]

Hello,

What about at least adding a change warning to Release Notes?

We do have software which depends on the possibility to actually reset a 
connection instead of closing it cleanly (think FTP data connection proxy), 
luckily in our case it’s blocking so we would not be affected, but I can 
imagine others need that on a channel as well.

Gruss
Bernd
--
http://bernd.eckenfels.net

From: net-dev <net-dev-boun...@openjdk.java.net> on behalf of Alan Bateman 
<alan.bate...@oracle.com>
Sent: Friday, May 11, 2018 9:34:06 PM
To: Norman Maurer
Cc: nio-...@openjdk.java.net; OpenJDK Network Dev list
Subject: Re: Behaviour of SocketChannelImpl.close() in Java11 (ea+12)

(cc'ing nio-dev as as this is asking about SocketChannel).

On 11/05/2018 19:10, Norman Maurer wrote:
> Hi all,
>
> I recently started to test Netty [1] with Java11 and found that we
> have two tests that are currently failing due some changes in Java 11
> compared to earlier versions.
>
> I wanted to get your thoughts on the behaviour changes:
>
> 1) SocketChannelImpl.close() will trigger shutdown(…,SHUT_WR) if the
> channel is connected before do the actual close(…).
>
> This is different to earlier version where it was just closed via
> close(…). We noticed this because we have a unit test that basically
> set SO_LINGER 0 and verifies that the remote peer sees a ECONNRESET
> when channel is closed. This is not the case here anymore as the
> shutdown will cause an EOF.
> I wonder depending on the connection reset is just plain wrong from
> our side as its an implementation detail, but at least it was super
> surprising to me that a shutdown(…) was called during the close
> operation. Especially as shutdownOutput() is exposed directly as well.
>
>
> 2) SocketChannelImpl.close() will not directly close the fd but add it
> to a queue that will be processed by the Selector.
>
> Again this is different to earlier versions and had the effect that
> one test failed that expected that the fd is really closed when
> close() returns.
>
If I read this correctly, #1 and #2 are asking about closing a
SocketChannel that is registered with a Selector. If registered with a
Selector then the channel must be configured non-blocking.

I'll start with #2 as closing a SocketChannel registered with a Selector
has always delayed releasing the file descriptor until it has been
flushed from all Selectors. If the Netty tests are monitoring the file
descriptor count (maybe with the management API?) then you shouldn't see
a difference. If #2 is about whether the peer sees a graceful close or a
TCP reset then the behavior should be the same as older releases too,
except when the linger-on-close socket option is enabled, which leads to
your question #1.

On #1, then one initial thing to point out is that SO_LINGER is only
specified for sockets configured in blocking mode. From the javadoc:

"This socket option is intended for use with sockets that are configured
in blocking mode only. The behavior of the close method when this option
is enabled on a non-blocking socket is not defined."

You are correct that there is a behavior difference in JDK 11 when
enabling this socket option on a SocketChannel configured non-blocking
and then closing it when it is registered with one or more Selector.
That behavior difference arises because close in JDK 10 (and older)
always "pre-closed" (essentially a dup2) the file descriptor whereas JDK
11 does not do this when the channel is configured non-blocking. The two
phase close trick is needed for channels in blocking mode, not so for
channels configured non-blocking where it has always been very
problematic to switch the file descriptor whilst registered with a Selector.

As regards the half close / shutdown when registered with a Selector
then this is so that the peer detects the connection shutdown. The peer
otherwise not observe the shutdown until the channel is flushed from the
Selector.

I'm in two minds as to whether we should do anything to try to restore
"not defined" behavior. We could potentially skip the shutdown when the
linger-on-close socket option is enabled. That would at least allow
tests that exercise TCP resets to continue to work, assuming the channel
is flushed promptly from all Selectors that it is registered with.

-Alan

Re: Behaviour of SocketChannelImpl.close() in Java11 (ea+12)

[Norman Maurer]

> On 11. May 2018, at 21:34, Alan Bateman  wrote:
> 
> (cc'ing nio-dev as as this is asking about SocketChannel).
> 
> On 11/05/2018 19:10, Norman Maurer wrote:
>> Hi all,
>> 
>> I recently started to test Netty [1] with Java11 and found that we have two 
>> tests that are currently failing due some changes in Java 11 compared to 
>> earlier versions.
>> 
>> I wanted to get your thoughts on the behaviour changes:
>> 
>> 1) SocketChannelImpl.close() will trigger shutdown(…,SHUT_WR) if the channel 
>> is connected before do the actual close(…).
>> 
>> This is different to earlier version where it was just closed via close(…). 
>> We noticed this because we have a unit test that basically set SO_LINGER 0 
>> and verifies that the remote peer sees a ECONNRESET when channel is closed. 
>> This is not the case here anymore as the shutdown will cause an EOF.
>> I wonder depending on the connection reset is just plain wrong from our side 
>> as its an implementation detail, but at least it was super surprising to me 
>> that a shutdown(…) was called during the close operation. Especially as 
>> shutdownOutput() is exposed directly as well.
>> 
>> 
>> 2) SocketChannelImpl.close() will not directly close the fd but add it to a 
>> queue that will be processed by the Selector.
>> 
>> Again this is different to earlier versions and had the effect that one test 
>> failed that expected that the fd is really closed when close() returns.
>> 
> If I read this correctly, #1 and #2 are asking about closing a SocketChannel 
> that is registered with a Selector. If registered with a Selector then the 
> channel must be configured non-blocking.

You are reading it correctly …


> 
> I'll start with #2 as closing a SocketChannel registered with a Selector has 
> always delayed releasing the file descriptor until it has been flushed from 
> all Selectors. If the Netty tests are monitoring the file descriptor count 
> (maybe with the management API?) then you shouldn't see a difference. If #2 
> is about whether the peer sees a graceful close or a TCP reset then the 
> behavior should be the same as older releases too, except when the 
> linger-on-close socket option is enabled, which leads to your question #1.

I should have also mentioned that this test uses SO_LINGER 0 and so is also 
related to 1) as well a bit but not 100 %. The problem basically is that before 
after we called SocketChannel.close() the write on the peer SocketChannel 
always failed after the close() call returned which seems to not be the case 
anymore.

> 
> On #1, then one initial thing to point out is that SO_LINGER is only 
> specified for sockets configured in blocking mode. From the javadoc:
> 
> "This socket option is intended for use with sockets that are configured in 
> blocking mode only. The behavior of the close method when this option is 
> enabled on a non-blocking socket is not defined.”

Interesting enough I never noticed this javadoc and just assumed it would work 
the same way as when I just do it via C, which also works for non-blocking 
sockets (at least with SO_LINGER 0).

> 
> You are correct that there is a behavior difference in JDK 11 when enabling 
> this socket option on a SocketChannel configured non-blocking and then 
> closing it when it is registered with one or more Selector. That behavior 
> difference arises because close in JDK 10 (and older) always "pre-closed" 
> (essentially a dup2) the file descriptor whereas JDK 11 does not do this when 
> the channel is configured non-blocking. The two phase close trick is needed 
> for channels in blocking mode, not so for channels configured non-blocking 
> where it has always been very problematic to switch the file descriptor 
> whilst registered with a Selector.
> 
> As regards the half close / shutdown when registered with a Selector then 
> this is so that the peer detects the connection shutdown. The peer otherwise 
> not observe the shutdown until the channel is flushed from the Selector.
> 
> I'm in two minds as to whether we should do anything to try to restore "not 
> defined" behavior. We could potentially skip the shutdown when the 
> linger-on-close socket option is enabled. That would at least allow tests 
> that exercise TCP resets to continue to work, assuming the channel is flushed 
> promptly from all Selectors that it is registered with.

As the java docs clearly state its “undefined” for non-blocking (which I never 
noticed)  I am completely happy either way. I was just surprised about the 
change in behaviour and wanted to bring it up as others may also be surprised.


> 
> -Alan

Thanks
Norman

Re: Behaviour of SocketChannelImpl.close() in Java11 (ea+12)

[Alan Bateman]

(cc'ing nio-dev as as this is asking about SocketChannel).

On 11/05/2018 19:10, Norman Maurer wrote:

Hi all,

I recently started to test Netty [1] with Java11 and found that we 
have two tests that are currently failing due some changes in Java 11 
compared to earlier versions.


I wanted to get your thoughts on the behaviour changes:

1) SocketChannelImpl.close() will trigger shutdown(…,SHUT_WR) if the 
channel is connected before do the actual close(…).


This is different to earlier version where it was just closed via 
close(…). We noticed this because we have a unit test that basically 
set SO_LINGER 0 and verifies that the remote peer sees a ECONNRESET 
when channel is closed. This is not the case here anymore as the 
shutdown will cause an EOF.
I wonder depending on the connection reset is just plain wrong from 
our side as its an implementation detail, but at least it was super 
surprising to me that a shutdown(…) was called during the close 
operation. Especially as shutdownOutput() is exposed directly as well.



2) SocketChannelImpl.close() will not directly close the fd but add it 
to a queue that will be processed by the Selector.


Again this is different to earlier versions and had the effect that 
one test failed that expected that the fd is really closed when 
close() returns.


If I read this correctly, #1 and #2 are asking about closing a 
SocketChannel that is registered with a Selector. If registered with a 
Selector then the channel must be configured non-blocking.


I'll start with #2 as closing a SocketChannel registered with a Selector 
has always delayed releasing the file descriptor until it has been 
flushed from all Selectors. If the Netty tests are monitoring the file 
descriptor count (maybe with the management API?) then you shouldn't see 
a difference. If #2 is about whether the peer sees a graceful close or a 
TCP reset then the behavior should be the same as older releases too, 
except when the linger-on-close socket option is enabled, which leads to 
your question #1.


On #1, then one initial thing to point out is that SO_LINGER is only 
specified for sockets configured in blocking mode. From the javadoc:


"This socket option is intended for use with sockets that are configured 
in blocking mode only. The behavior of the close method when this option 
is enabled on a non-blocking socket is not defined."


You are correct that there is a behavior difference in JDK 11 when 
enabling this socket option on a SocketChannel configured non-blocking 
and then closing it when it is registered with one or more Selector. 
That behavior difference arises because close in JDK 10 (and older) 
always "pre-closed" (essentially a dup2) the file descriptor whereas JDK 
11 does not do this when the channel is configured non-blocking. The two 
phase close trick is needed for channels in blocking mode, not so for 
channels configured non-blocking where it has always been very 
problematic to switch the file descriptor whilst registered with a Selector.


As regards the half close / shutdown when registered with a Selector 
then this is so that the peer detects the connection shutdown. The peer 
otherwise not observe the shutdown until the channel is flushed from the 
Selector.


I'm in two minds as to whether we should do anything to try to restore 
"not defined" behavior. We could potentially skip the shutdown when the 
linger-on-close socket option is enabled. That would at least allow 
tests that exercise TCP resets to continue to work, assuming the channel 
is flushed promptly from all Selectors that it is registered with.


-Alan

Re: Behaviour of SocketChannelImpl.close() in Java11 (ea+12)

[Norman Maurer]

Sorry I just noticed this would better be asked on nio.dev. Will ask there.

Bye
Norman


> On 11. May 2018, at 20:10, Norman Maurer  wrote:
> 
> Hi all,
> 
> I recently started to test Netty [1] with Java11 and found that we have two 
> tests that are currently failing due some changes in Java 11 compared to 
> earlier versions.
> 
> I wanted to get your thoughts on the behaviour changes:
> 
> 1) SocketChannelImpl.close() will trigger shutdown(…,SHUT_WR) if the channel 
> is connected before do the actual close(…).
> 
> This is different to earlier version where it was just closed via close(…). 
> We noticed this because we have a unit test that basically set SO_LINGER 0 
> and verifies that the remote peer sees a ECONNRESET when channel is closed. 
> This is not the case here anymore as the shutdown will cause an EOF. 
> I wonder depending on the connection reset is just plain wrong from our side 
> as its an implementation detail, but at least it was super surprising to me 
> that a shutdown(…) was called during the close operation. Especially as 
> shutdownOutput() is exposed directly as well.
> 
> 
> 2) SocketChannelImpl.close() will not directly close the fd but add it to a 
> queue that will be processed by the Selector.
> 
> Again this is different to earlier versions and had the effect that one test 
> failed that expected that the fd is really closed when close() returns. 
> 
> 
> I worked around these differences via [2] but I wanted to ask if this is 
> expected.
> 
> 
> Java version:
> 
> java version "11-ea" 2018-09-25
> Java(TM) SE Runtime Environment 18.9 (build 11-ea+12)
> Java HotSpot(TM) 64-Bit Server VM 18.9 (build 11-ea+12, mixed mode)
> 
> 
> [1] http://netty.io 
> [2] https://github.com/netty/netty/pull/7926 
> 
> 
>