On Thu, 14 Nov 2024 04:56:12 GMT, Jaikiran Pai <j...@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. is it the case that a try-with-resource, and the implicit final declaration of s2, will maintain a strongly reachable reference ? Rather than having to declare s2 as a final member variable of Socket_getInputStream_read, as done with the reader socket s, or using a ReachabilityFence to maintain an extant reference. I don't see I terms of "cleanliness" the listener ServerSocket is implicitly released, rather than having an explicit close in a finally block for the outer try block, or being initialised in an outer try-with-resources ------------- PR Comment: https://git.openjdk.org/jdk/pull/22093#issuecomment-2476119599
