On Fri, 11 Jun 2021 16:27:07 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> There is the possibility for a race in the test, where the outbound socket >> buffer is still being filled, after 5 seconds, when the main test thread >> tries to close the resource scope. >> >> The test has been updated to set the send/receive buffer sizes in an >> attempt/hint to limit the accepted/connected socket buffer sizes. Actual >> buffer sizes in use will likely be larger due to TCP auto-tuning, but the >> hint typically reduces the overall scaled sizes. This is primarily to >> stabilize outstanding write operations. Additionally, to ameliorate the >> wait-for-writebuf-to-fill situation, the dumb sleep 5secs has been replaced >> with a heuristic that checks that the number of bytes written quiesces. With >> these changes, the test successfully passes several thousands of runs. > > test/jdk/java/foreign/channels/TestAsyncSocketChannels.java line 283: > >> 281: readNBytes(asc2, bytesWritten.get()); >> 282: assertTrue(scope.isAlive()); >> 283: awaitOutstandingWrites(outstandingWriteOps); > > An alternative would be to delegate the call to latch.countDown() to the > TestHandler subclass, and call it at the end of completed() only when > outstandingWriteOps.decrementAndGet() == 0; As it stands the latch is > released with the first call to `completed` (instead of being released with > the last one) - and that's what makes this method necessary. Strictly speaking it wasn't necessary to touch this area of code to resolve the failures that we're seeing - I decided to refactor this into a separate method to improve readability of the test logic. Here again, awaiting for completion is not strictly necessary, just good practice to ensure that all threads and operations associated with the test scenario complete before the next scenario. I'll see if this can be simplified as you suggest. ------------- PR: https://git.openjdk.java.net/jdk17/pull/30
