RE: 8199329: Remove code that attempts to read bytes after connection reset reported

[Langer, Christoph]

Hi Alan,

looks good to me, +1.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Alan Bateman
> Sent: Mittwoch, 14. März 2018 15:31
> To: OpenJDK Network Dev list <net-dev@openjdk.java.net>
> Subject: 8199329: Remove code that attempts to read bytes after connection
> reset reported
> 
> 
> Classic networking has some curious code to deal with connection resets.
> I needed to dig into ancient history to find the issues and original
> motivations.
> 
> When a connection reset is reported (usually ECONNRESET) then further
> attempts to read from the socket will typically return -1 (EOF). Classic
> networking papers over this so that further attempts with the socket
> APIs to read bytes will continue to throw SocketException "connection
> reset". Furthermore, it allows for cases where the platform can read
> bytes from the socket buffer after ECONNRESET has been reported. None of
> the main stream platforms do this and it's hard to imagine anything
> depending on such unpredictable behavior. I'm running into this odd code
> as part of cleanup to the read/write code paths and reducing them down
> to a single blocking call.
> 
> I would like to remove the legacy/undocumented behavior that attempts to
> read beyond the connection reset. The code to consistently throw
> IOException once ECONNRESET is returned is retained as it's possible
> that code relies on this.
> 
> The proposed changes are here:
>     http://cr.openjdk.java.net/~alanb/8199329/webrev/
> 
> All existing tests pass with these changes.
> 
> -Alan

Re: 8199329: Remove code that attempts to read bytes after connection reset reported

[Chris Hegarty]

On 14/03/18 14:30, Alan Bateman wrote:


Classic networking has some curious code to deal with connection resets. 
I needed to dig into ancient history to find the issues and original 
motivations.


When a connection reset is reported (usually ECONNRESET) then further 
attempts to read from the socket will typically return -1 (EOF). Classic 
networking papers over this so that further attempts with the socket 
APIs to read bytes will continue to throw SocketException "connection 
reset". Furthermore, it allows for cases where the platform can read 
bytes from the socket buffer after ECONNRESET has been reported. None of 
the main stream platforms do this and it's hard to imagine anything 
depending on such unpredictable behavior. I'm running into this odd code 
as part of cleanup to the read/write code paths and reducing them down 
to a single blocking call.


I would like to remove the legacy/undocumented behavior that attempts to 
read beyond the connection reset. The code to consistently throw 
IOException once ECONNRESET is returned is retained as it's possible 
that code relies on this.


The proposed changes are here:
http://cr.openjdk.java.net/~alanb/8199329/webrev/


Looks fine.

Curiously, I cannot find a test that checks the
read-after-reset-throws-SE, but your changes look good.

-Chris.

Re: 8199329: Remove code that attempts to read bytes after connection reset reported

[Claes Redestad]

Looks good to me.

/Claes

On 2018-03-14 15:30, Alan Bateman wrote:


Classic networking has some curious code to deal with connection 
resets. I needed to dig into ancient history to find the issues and 
original motivations.


When a connection reset is reported (usually ECONNRESET) then further 
attempts to read from the socket will typically return -1 (EOF). 
Classic networking papers over this so that further attempts with the 
socket APIs to read bytes will continue to throw SocketException 
"connection reset". Furthermore, it allows for cases where the 
platform can read bytes from the socket buffer after ECONNRESET has 
been reported. None of the main stream platforms do this and it's hard 
to imagine anything depending on such unpredictable behavior. I'm 
running into this odd code as part of cleanup to the read/write code 
paths and reducing them down to a single blocking call.


I would like to remove the legacy/undocumented behavior that attempts 
to read beyond the connection reset. The code to consistently throw 
IOException once ECONNRESET is returned is retained as it's possible 
that code relies on this.


The proposed changes are here:
   http://cr.openjdk.java.net/~alanb/8199329/webrev/

All existing tests pass with these changes.

-Alan

Re: 8199329: Remove code that attempts to read bytes after connection reset reported

[David Lloyd]

+1 from me (non-reviewer).  Semantically, it is impossible to
distinguish the difference between the OS dropping bytes when it
receives an RST compared to Java doing the same thing, so there is no
observable behavior change here AFAICT.

On Wed, Mar 14, 2018 at 9:30 AM, Alan Bateman  wrote:
>
> Classic networking has some curious code to deal with connection resets. I
> needed to dig into ancient history to find the issues and original
> motivations.
>
> When a connection reset is reported (usually ECONNRESET) then further
> attempts to read from the socket will typically return -1 (EOF). Classic
> networking papers over this so that further attempts with the socket APIs to
> read bytes will continue to throw SocketException "connection reset".
> Furthermore, it allows for cases where the platform can read bytes from the
> socket buffer after ECONNRESET has been reported. None of the main stream
> platforms do this and it's hard to imagine anything depending on such
> unpredictable behavior. I'm running into this odd code as part of cleanup to
> the read/write code paths and reducing them down to a single blocking call.
>
> I would like to remove the legacy/undocumented behavior that attempts to
> read beyond the connection reset. The code to consistently throw IOException
> once ECONNRESET is returned is retained as it's possible that code relies on
> this.
>
> The proposed changes are here:
>http://cr.openjdk.java.net/~alanb/8199329/webrev/
>
> All existing tests pass with these changes.
>
> -Alan



-- 
- DML

8199329: Remove code that attempts to read bytes after connection reset reported

[Alan Bateman]

Classic networking has some curious code to deal with connection resets. 
I needed to dig into ancient history to find the issues and original 
motivations.


When a connection reset is reported (usually ECONNRESET) then further 
attempts to read from the socket will typically return -1 (EOF). Classic 
networking papers over this so that further attempts with the socket 
APIs to read bytes will continue to throw SocketException "connection 
reset". Furthermore, it allows for cases where the platform can read 
bytes from the socket buffer after ECONNRESET has been reported. None of 
the main stream platforms do this and it's hard to imagine anything 
depending on such unpredictable behavior. I'm running into this odd code 
as part of cleanup to the read/write code paths and reducing them down 
to a single blocking call.


I would like to remove the legacy/undocumented behavior that attempts to 
read beyond the connection reset. The code to consistently throw 
IOException once ECONNRESET is returned is retained as it's possible 
that code relies on this.


The proposed changes are here:
   http://cr.openjdk.java.net/~alanb/8199329/webrev/

All existing tests pass with these changes.

-Alan