RFR: 8334719: (se) Deferred close of SelectableChannel may result in a Selector doing the final close before concurrent I/O on channel has completed [v5]

[Daniel Fuchs]

On Wed, 26 Jun 2024 13:32:10 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>>> I assume it doesn't need to be a public static field, it could be a private instance field.
>> 
>> It should have been a private field. It's an oversight that I left it as public in a recent update. I'll fix that.
>> 
>>> I think that would remove the need for the beforeAll/afterAll as it would allow the test to just set the delay for the ISA used by the test.
>> 
>> The important detail here is that to consistently reproduce this issue both in send and receive methods of the DatagramChannelImpl, the delay has to be introduced not just in the ISA constructed by the test methods, but also in the ISA constructed by the DatagramChannelImpl implementation's decoding of native addresses to ISA. That decoding of ISA happens deep within the receive operation after it has acquired the relevant lock and that's the place where we want to slow things down so that a concurrent thread can then enter and finish close the channel which was in deferred close state.
>> 
>> If we wanted to reproduce the issue only for send, then constructing the delayed ISA in the test method should have been enough (because send would just call the ISA.getPort() on the ISA passed by the test method). But I also wanted to be sure that there was a way to reproduce (and thus verify the fix) for receive too (which internally calls the ISA constructor at least once upon receiving a datagram and converting the native source address to the ISA). That was the reason why I chose to have this in the beforeAll/afterAll, to keep it relatively simple.
>
> Okay, I think we can live with that.

Technically - it may be set in only one thread but it's consulted in many other threads. However there should be already enough full memory barreers around to ensure that the other threads will see the modifications, especially since these threads are started after the modification was made. So it shouldn't be an issue for the test in this form.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19879#discussion_r1654985754