RFR: 8343877: Test AsyncClose.java intermittent fails - Socket.getInputStream().read() wasn't preempted

[Jaikiran Pai]

On Thu, 14 Nov 2024 09:11:58 GMT, SendaoYan <syan at openjdk.org> wrote:

>> Can I please get a review of this test-only fix which addresses the intermittent failure in AsyncClose test as noted in https://bugs.openjdk.org/browse/JDK-8343877?
>> 
>> The `FileDescriptor` of the `Socket` instance that gets returned from a call to `ServerSocket.accept()` gets enrolled with the `java.lang.ref.Cleaner` so that the underlying file descriptor of the newly accepted socket gets closed.
>> 
>> The `Socket_getInputStream_read` and even the `Socket_getOutputStream_write` tests don't hold on to the `Socket` instance returned by the `accept()` call until the concurrent `Thread` that reads (or writes) on the client side of the socket. This is an oversight in the test and this can lead to a case where the GC upon noticing that the returned `Socket` instance isn't referenced may garbage collect that instance and thus the `Cleaner` may end up closing the underlying file descriptor. When that happens, the expectations of the test don't hold and thus the test fails as noted in the linked issue.
>> 
>> The commit in this PR updates these 2 tests to use a try-with-resources for the returned `Socket` instance and that way enforce reachability of the `Socket` instance until the end of the try block. This should prevent the GC from garbage collecting the instance and at the same time `close()` the returned `Socket` in a deterministic manner.
>> 
>> No explicit `java.lang.ref.Reference.reachabilityFence()` should be required here given what the JLS specifies about try-with-resources and reachability in section 14.20.3 https://docs.oracle.com/javase/specs/jls/se23/html/jls-14.html#jls-14.20.3.
>> 
>> These tests continue to pass after this change with a test repeat of 50. tier2 too passes without any failures.
>
> After git apply the patch from https://github.com/openjdk/jdk/pull/22093 and run the test 10k times, all test passed.

Thank you all for the reviews and thanks to Daniel, Mark and @sendaoYan for the help during the investigation of this failure. I'll go ahead and integrate this shortly.

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

PR Comment: https://git.openjdk.org/jdk/pull/22093#issuecomment-2476388046