8223353: (ch) Change channel close implementation to not wait for I/O threads

[Daniel Fuchs]

Hi Alan,

The changes seem reasonable to me. It looks like a nice
simplification. I guess the JIRA issue will need a noreg-cleanup
label. I assume you have verified that the tests have remained
stable?

best regards,

-- daniel

On 05/05/2019 20:00, Alan Bateman wrote:
> I'd like to refactor the SelectableChannel close implementations so that 
> the close methods don't wait for I/O operations to abort when closing a 
> channel in blocking mode.
> 
> As I'm sure we all know here, closing a channel is complicated because 
> it requires coordinating with threads doing I/O operations. It also 
> requires coordinating with selection operations as the channel may be 
> registered with one or more Selectors. As things currently stand, 
> closing a channel in blocking mode with threads blocked in I/O 
> operations involves:
> 
> 1. pre-close (the dup2 trick to close the connection but not release the 
> file descriptor)
> 2. signal the threads in blocking I/O operations so that they abort with 
> EINTR
> 3. wait for the threads to complete/abort the I/O operation
> 4. close the file descriptor (if not registered with a Selector - not 
> usually possible with channels configured in blocking mode but there are 
> convoluted scenarios to consider).
> 
> For comparison, the non-blocking case is a simple lock/unlock to ensure 
> that all I/O operations have completed and then closing the file 
> descriptor when not registered with a Selector. If registered with a 
> Selector then the close is deferred until its key is flushed from the 
> last Selector that it is registered with.
> 
> The proposed change is to eliminate #3 above and have the last I/O 
> operation to complete do the final close. This simplifies the close nd 
> means that I/O operation in progress and registered with Selector cases 
> are handled consistently. The change should not be observable to code 
> using these APIs as the pre-close will close the connection as before.
> 
> One benefit of the change is that is Thread.interrupt doesn't wait. A 
> second benefit is that we move stateLock in the implementations back to 
> being a built-in monitor, the condition object used to await/signal, 
> goes away.
> 
> The webrev with the proposed changes is here. The changes to stateLock 
> are purely mechanical, everything else is the refactor of the 
> implCloseSelectableChannel implementations.
> 
> http://cr.openjdk.java.net/~alanb/8223353/0/webrev/
> 
> -Alan