On Thu, 12 Mar 2026 12:40:46 GMT, SendaoYan <[email protected]> wrote:
>> Hi all,
>>
>> Test javax/net/ssl/Stapling/HttpsUrlConnClient.java fails "Read timed out"
>> intermittently, espically the system is overload. This PR make the several
>> timeout value receive and multiply the jtreg timeout factor. This will make
>> test more robustness.
>> Change has been verified locally by run the test 1000 times simultancely.
>
> SendaoYan has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Optimize import
> - Fix "Server not ready" error in createPKI
test/jdk/javax/net/ssl/Stapling/HttpsUrlConnClient.java line 104:
> 102:
> 103: // Turn on TLS debugging
> 104: static boolean debug = true;
I think this either could be removed entirely and just make a `javax.net.debug
` option in `@run` or `static boolean debug =
Boolean.getBoolean("test.debug");`. Could you please change it ?
I personally prefer the first option here, but ultimately it’s up to you.
test/jdk/javax/net/ssl/Stapling/HttpsUrlConnClient.java line 111:
> 109: * as to which side should be the main thread.
> 110: */
> 111: static boolean separateServerThread = true;
Is there a difference? Do you think this might be 2 options in 2 different
`@run`s?
test/jdk/javax/net/ssl/Stapling/HttpsUrlConnClient.java line 345:
> 343: HtucSSLSocketFactory sockFac = new
> HtucSSLSocketFactory(cliParams);
> 344: HttpsURLConnection.setDefaultSSLSocketFactory(sockFac);
> 345: URL location = new URL("https://localhost:"; + serverPort);
Could you please change this to use the `InetAddress.getLoopbackAddress()`
address?
test/jdk/javax/net/ssl/Stapling/HttpsUrlConnClient.java line 348:
> 346: HttpsURLConnection tlsConn =
> 347: (HttpsURLConnection)location.openConnection();
> 348: tlsConn.setConnectTimeout((int)Utils.adjustTimeout(5000));
Nit: for these timeouts, why not use the same line style as on line 635 with
specifying `SECONDS`?
test/jdk/javax/net/ssl/Stapling/HttpsUrlConnClient.java line 752:
> 750: boolean enabled = true;
> 751: int cacheSize = 256;
> 752: int cacheLifetime = 3600;
Shouldn’t all times be adjusted then? Does it make any difference, what do you
think?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30212#discussion_r2936631318
PR Review Comment: https://git.openjdk.org/jdk/pull/30212#discussion_r2936636188
PR Review Comment: https://git.openjdk.org/jdk/pull/30212#discussion_r2936625242
PR Review Comment: https://git.openjdk.org/jdk/pull/30212#discussion_r2936624602
PR Review Comment: https://git.openjdk.org/jdk/pull/30212#discussion_r2936626385
