RFR: 8278326: Socket close is not thread safe and other cleanup

[Alan Bateman]

On Tue, 10 Jan 2023 13:37:53 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> ServerSocket.implAccept is intended to be called with a newly created, and unbound, Socket. We looked at this topic in JDK 13 (see JDK-8225431 and related issues) and decided the behavior is unspecified for silly/broken cases where it is called with a Socket in any other state. It should probably have been specified to throw ISE when introduced in JDK 1.1 but we decided there wasn't enough motive to try to fix the spec for that. This PR touches this area again because it interacts with Socket.close.
>> 
>> So the question is what is the JDK behavior when calling implAccept with a connected Socket. A Socket can't have two connections at the same time. The JDK behavior is to attach the SocketImpl for the newly accepted connection to the Socket. This breaks the association from the Socket to the previous SocketImpl. This PR doesn't change that. It does re-visit the question as to what to do with the now orphaned SocketImpl. Should it be discarded, in which case it may be a resource leak, or should it be closed?  The former seems preferable, in which case the read/write methods of streams handed out before calling implAccept will throw IOException. Your question is what happens if the input or output stream close method is called? The spec is that the close method closes the Socket so I've left this as is, it's not changed in this PR. The only real change here is that calling getInputStream/getOutputStream after accepting the new connection will return streams to the newly accepted conn
 ection. That seems the most sensible thing to do for this crazy case.
>
> Thank you for that detailed explanation, Alan. That helped.
> 
>> Should it be discarded, in which case it may be a resource leak, or should it be closed? The former seems preferable, in which case the read/write methods of streams handed out before calling implAccept will throw IOException.
> 
> I'm guessing that is a typo and you actually mean "The latter seems preferable ..."? The implementation in this PR  does the latter and closes the previous orphaned SocketImpl and I think that's the right thing to do.

Sorry, I meant the former as closing seems preferable,

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

PR: https://git.openjdk.org/jdk/pull/11863