On Fri, 11 Jun 2021 15:26:50 GMT, Chris Hegarty <che...@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. Looks good but you could make the logic simpler by calling latch.countDown() in the last call to completed(), instead of doing it in the first one. 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. ------------- Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk17/pull/30
