On Thu, 6 Mar 2025 12:28:19 GMT, Fernando Guallini <[email protected]>
wrote:
>> The following tests are marked with @ignore (not running):
>>
>> - sun/security/ssl/SSLSocketImpl/SetClientMode.java: it checks that setting
>> the clientMode after the handshake has begun is not permitted, but this was
>> failing intermittently due to a race condition, it was possible that
>> SetClientMode was called before the client socket was updated with handshake
>> isNegotiated = true. The fix is to introduce a latch to sync between client
>> and main threads. Included additional refactoring to ensure test stability.
>>
>> - sun/security/ssl/SSLSocketImpl/NonAutoClose.java: This test should only
>> run in TLS <= 1.2, as TLSv1.3 changes the behaviour of close_notify.
>> Included additional refactoring to ensure test stability.
>>
>> Executed both tests 10K times, no test flakiness found
>
> Fernando Guallini has updated the pull request incrementally with one
> additional commit since the last revision:
>
> SSLContextTemplate and using asserts
test/jdk/sun/security/ssl/SSLSocketImpl/NonAutoClose.java line 135:
> 133: */
> 134: System.out.println("Waiting for server ready");
> 135: if (!SERVER_READY.await(5, TimeUnit.SECONDS)) {
Please see my other comment about `await()`.
test/jdk/sun/security/ssl/SSLSocketImpl/SetClientMode.java line 100:
> 98: connectedSocket.getSession();
> 99:
> 100: if (!HANDSHAKE_COMPLETE.await(5, TimeUnit.SECONDS)) {
The test will be more robust if we do a simple `await()` call here. On some
busy systems even 5 seconds may not be enough. JTEG tests have a default
timeout of 2 minutes, so it won't wait forever in any case.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23898#discussion_r2000018849
PR Review Comment: https://git.openjdk.org/jdk/pull/23898#discussion_r2000014224