Re: NIO based SocketImpl to replace legacy PlainSocketImpl

[Ismael Juma]

Nice to see this, hope it works out.

Ismael

On Fri, Jan 25, 2019, 6:11 AM Alan Bateman 
> I've created a branch in the sandbox, named "niosocketimpl-branch", with
> a prototype SocketImpl implementation based on the infrastructure in
> sun.nio.ch package that supports the NIO channel implementations. The
> branch also includes the changes to java.net.Socket and ServerSocket to
> use this SocketImpl by default.
>
> There are several motivations for this prototype: The existing
> PlainSocketImpl is not fit for purpose in the potential future world of
> fibers that park instead of blocking carrier threads in syscalls. As I'm
> sure most people are on net-dev are aware, PlainSocketImpl is horrible
> to maintain due to the complexity of the code paths in its native
> methods. Replacing PlainSocketImpl, along with its associates
> SocketInputStream and SocketOutputStream, bring other benefits such as
> not needing to use the thread stack for I/O and not needing the fd table
> data structure to support asynchronous close.
>
> We have to be a cautious about replacing the PlainSocket/SIS/SOS after
> 20 years of service. It is likely that there is behavior in unspecified
> areas and corner case scenarios that existing applications or libraries
> may depend upon. There is performance work to do but it's possible there
> are cases where the performance might degrade, e.g. many years ago there
> were attempts to fix a synchronization issue in PlainSocketImpl that
> lead to complaints from usages involving hundreds of threads blocking on
> ServerSocket::accept. To that end, we are planning to evolve the
> prototype to allow the old and new SocketImpls to co-exist, maybe
> switched by a system property or some other means. Going there may
> require a bit yak shaving, for example in the SOCKS and HTTP proxy
> implementations as they need to be re-worked to forward rather than
> sub-class. If the prototype goes further then the idea is that both
> implementations could be included in the JDK for a few releases before
> removing the old code.
>
> The prototype doesn't have a replacement for PlainDagramSocketImpl at
> this time. There are a few issues around how legacy MulticastSocket
> behaves that will take a bit of time to sort out. Ideally its
> replacement will use the same infrastructure as the DatgramChannel
> implementation as that supports modern muilticasting features and is a
> lot easier to maintain.
>
> For now, we (= Michael McMahon and myself for now) will iterate on this
> prototype in the sandbox. If we go forward with it then we'll likely
> create a JEP.
>
> -Alan
>

Re: NIO based SocketImpl to replace legacy PlainSocketImpl

[Bernd Eckenfels]

How will that look like on Windows, will it use IO Completion Ports? I guess 
scalability becomes much more of an issue with typical thousands of classic 
sockets.

What’s the expected performance of this? The blocking IO had a lot less latency 
compared to Channels, is there some regression expected or is a goal to also 
improve latency compared with the classic implementation?

Gruss
Bernd

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


Von: net-dev  im Auftrag von Ismael Juma 

Gesendet: Dienstag, Januar 29, 2019 11:05 AM
An: Alan Bateman
Cc: nio-...@openjdk.java.net; OpenJDK Network Dev list
Betreff: Re: NIO based SocketImpl to replace legacy PlainSocketImpl

Nice to see this, hope it works out.

Ismael

On Fri, Jan 25, 2019, 6:11 AM Alan Bateman 
mailto:alan.bate...@oracle.com> wrote:

I've created a branch in the sandbox, named "niosocketimpl-branch", with
a prototype SocketImpl implementation based on the infrastructure in
sun.nio.ch package that supports the NIO channel 
implementations. The
branch also includes the changes to java.net.Socket and ServerSocket to
use this SocketImpl by default.

There are several motivations for this prototype: The existing
PlainSocketImpl is not fit for purpose in the potential future world of
fibers that park instead of blocking carrier threads in syscalls. As I'm
sure most people are on net-dev are aware, PlainSocketImpl is horrible
to maintain due to the complexity of the code paths in its native
methods. Replacing PlainSocketImpl, along with its associates
SocketInputStream and SocketOutputStream, bring other benefits such as
not needing to use the thread stack for I/O and not needing the fd table
data structure to support asynchronous close.

We have to be a cautious about replacing the PlainSocket/SIS/SOS after
20 years of service. It is likely that there is behavior in unspecified
areas and corner case scenarios that existing applications or libraries
may depend upon. There is performance work to do but it's possible there
are cases where the performance might degrade, e.g. many years ago there
were attempts to fix a synchronization issue in PlainSocketImpl that
lead to complaints from usages involving hundreds of threads blocking on
ServerSocket::accept. To that end, we are planning to evolve the
prototype to allow the old and new SocketImpls to co-exist, maybe
switched by a system property or some other means. Going there may
require a bit yak shaving, for example in the SOCKS and HTTP proxy
implementations as they need to be re-worked to forward rather than
sub-class. If the prototype goes further then the idea is that both
implementations could be included in the JDK for a few releases before
removing the old code.

The prototype doesn't have a replacement for PlainDagramSocketImpl at
this time. There are a few issues around how legacy MulticastSocket
behaves that will take a bit of time to sort out. Ideally its
replacement will use the same infrastructure as the DatgramChannel
implementation as that supports modern muilticasting features and is a
lot easier to maintain.

For now, we (= Michael McMahon and myself for now) will iterate on this
prototype in the sandbox. If we go forward with it then we'll likely
create a JEP.

-Alan

Re: NIO based SocketImpl to replace legacy PlainSocketImpl

[Alan Bateman]

On 29/01/2019 10:21, Bernd Eckenfels wrote:
How will that look like on Windows, will it use IO Completion Ports? I 
guess scalability becomes much more of an issue with typical thousands 
of classic sockets.


What’s the expected performance of this? The blocking IO had a lot 
less latency compared to Channels, is there some regression expected 
or is a goal to also improve latency compared with the classic 
implementation?


There's nothing specific to Windows in this. Reading and writing ends up 
doing WSARecv/WSASend on sockets configured blocking, non-blocking with 
using timeouts. So no I/O completion ports in the picture, also no 
channels either. There is a lot of old code using Socket/ServerSocket so 
we don't want to regress performance.


-Alan

RFR [13] 8217976: test/jdk/java/net/httpclient/websocket/WebSocketProxyTest.java fails intermittently

[Chris Hegarty]

WebSocketProxyTest is a new test that was added recently. It fails once
in every few hundred runs. The test uses a preexisting test-only proxy
server. There is a race when closing the server; the close method
iterates over all connections while another thread may be adding a new
connection. The solution proposed here is to take a copy of the
connection list and iterate over it, rather than the connection list
itself.  ( I also did a little clean up and added some consistency to
proxy debug messages, since it was more difficult to reason about the
test's behavior from its log than it should have been )

http://cr.openjdk.java.net/~chegar/8217976/webrev.00/

-Chris.

Re: RFR [13] 8217976: test/jdk/java/net/httpclient/websocket/WebSocketProxyTest.java fails intermittently

[Daniel Fuchs]

Hi Chris,


 116 List localList = List.copyOf(connections);

I think it is still possible for CME to happen during the copyOf
operation, even though it can reduce the window in which that might
happen.

The best would be to use CopyOnWriteArrayList instead
of synchronizedList(). Otherwise you'd still need to copy the list
within a synchronize(connections) { } block to prevent CME.

best regards,

-- daniel
On 29/01/2019 12:15, Chris Hegarty wrote:

WebSocketProxyTest is a new test that was added recently. It fails once
in every few hundred runs. The test uses a preexisting test-only proxy
server. There is a race when closing the server; the close method
iterates over all connections while another thread may be adding a new
connection. The solution proposed here is to take a copy of the
connection list and iterate over it, rather than the connection list
itself.  ( I also did a little clean up and added some consistency to
proxy debug messages, since it was more difficult to reason about the
test's behavior from its log than it should have been )

http://cr.openjdk.java.net/~chegar/8217976/webrev.00/

-Chris.

Re: RFR [13] 8217976: test/jdk/java/net/httpclient/websocket/WebSocketProxyTest.java fails intermittently

[Chris Hegarty]

> On 29 Jan 2019, at 12:31, Daniel Fuchs  wrote:
> 
> ...
> The best would be to use CopyOnWriteArrayList instead
> of synchronizedList(). Otherwise you'd still need to copy the list
> within a synchronize(connections) { } block to prevent CME.

Agreed. Should be good enough for test cleanup.

http://cr.openjdk.java.net/~chegar/8217976/webrev.01/

-Chris.

Re: RFR [13] 8217976: test/jdk/java/net/httpclient/websocket/WebSocketProxyTest.java fails intermittently

[Daniel Fuchs]

Looks good.

While you're at it you could make:
 123 CopyOnWriteArrayList connections;
final. No need for a new webrev.

best regards,

-- daniel

On 29/01/2019 12:53, Chris Hegarty wrote:

Agreed. Should be good enough for test cleanup.

http://cr.openjdk.java.net/~chegar/8217976/webrev.01/

-Chris.

RFR (testbug): 8216562: UnknownBodyLength sometimes fails due to "Connection reset by peer"

[Daniel Fuchs]

Hi,

Please find below a fix for:

8216562: UnknownBodyLength sometimes fails due to
 "Connection reset by peer"
https://bugs.openjdk.java.net/browse/JDK-8216562

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8216562/webrev.00/

My previous attempt at at fixing this test proved
unsuccessful, as I was still seeing some very infrequent
failures - caused by connection reset, with https.

The present fix does 3 things:

1. increase the SO_LINGER time, to reduce the risk of
   generating a reset (1s -> 30s).
2. keep the accepted socket around (server side) until the
   client finishes - to avoid it to be prematurely gc'ed
   and its file descriptor released.
3. calls shutdownOutput() - but delay closing the socket
   until the client finishes. This should also reduce the
   risk of generating a reset.

with this, I haven't been able to reproduce the issue yet.
I believe it does fix it - though I'm afraid only time will
tell...

best regards,

-- daniel