On Tue, 13 Jun 2023 14:24:20 GMT, Matthew Donovan <mdono...@openjdk.org> wrote:
>> This PR improves the reliability of the BlockedAsyncClose test by addressing >> an edge case/race condition between the two test threads. The purpose of the >> test is to verify that an SSLSocket can be closed if a thread is blocked in >> a write operation. >> >> The test starts a "write" thread that writes data to a socket until the >> output buffer was filled, causing the write operation to block. The main >> thread then calls `SSLSocket.close()`. The original code used >> `Thread.sleep(1000)` to wait for the write-thread to block. However, 1 >> second isn't always long enough and if the write-thread isn't blocked and >> the output buffer is full (or almost full), the `socket.close()` call may >> block when it tries to send the close_notify alert. This is the condition >> that caused this bug. >> >> My change uses a Lock to determine if the write thread is blocked. In the >> write thread, the lock creates a critical section around the `write()` call. >> The main thread uses `tryLock()` with a timeout to determine that the >> write() call is taking too long and thus likely blocked. >> >> While there, I also updated the test to use the SSLContextTemplate class. > > Matthew Donovan has updated the pull request incrementally with one > additional commit since the last revision: > > Fixed whitespace. > > Co-authored-by: Andrey Turbanov <turban...@gmail.com> LGTM. Some OSes spontaneously increase the buffer size after a few seconds, unblocking blocked write operations. We also use 10 seconds limit in networking tests, seems to work pretty well. See https://github.com/openjdk/jdk/blob/9e4fc568a6f1a93c84a84d6cc5220c6eb4e546a5/test/jdk/java/net/httpclient/websocket/PendingOperations.java#L43-L47 ------------- Marked as reviewed by djelinski (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14378#pullrequestreview-1541088196
