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]

[Jaikiran Pai]

On Wed, 26 Jun 2024 11:28:48 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> test/jdk/java/nio/channels/Selector/DeferredClose/java.base/java/net/InetSocketAddress.java line 34:
>> 
>>> 32:     private static final long serialVersionUID = 5076001401234631237L;
>>> 33: 
>>> 34:     public static boolean enableDelay;
>> 
>> should it be volatile?
>
> The main thing for this test is that it's easy to maintain because someone looking at the test in a few years (maybe because of a test failure) will want to understand it quickly. I assume it doesn't need to be a public static field, it could be a private instance field. 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. I don't want to water Jai's time on too many iterations on this test, I think the main thing is that it is reliable and easy to maintain.

Hello Daniel, I don't think it's needed. We set this to `true` (only) once in a `@BeforeAll` which gets called by one single thread. The subsequent multiple threads will only get created and launched as part of the test methods after the `@BeforeAll` has finished, so I believe they would all correctly see enableDelay as true.

After that, we only update that `enableDelay` value after all tests are done, so that too should be fine.

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

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