On Thu, 8 Jun 2023 14:25:26 GMT, Matthew Donovan <[email protected]> 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.
test/jdk/sun/security/ssl/SSLSocketImpl/BlockedAsyncClose.java line 90:
> 88: // if the writeLock is not released by the other thread within 10
> 89: // seconds it is probably blocked, and we can try to close the
> socket
> 90: while(writeLock.tryLock(10, TimeUnit.SECONDS)) {
Suggestion:
while (writeLock.tryLock(10, TimeUnit.SECONDS)) {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14378#discussion_r1228181139
