On Thu, 14 Aug 2025 12:14:48 GMT, Mikhail Yankelevich
<[email protected]> wrote:
>> * fully automated the test
>> * removed the race condition
>> * client on a thread and server on a thread options are now run together
>> automatically
>
> Mikhail Yankelevich has updated the pull request incrementally with one
> additional commit since the last revision:
>
> addressing comments
test/jdk/sun/net/www/protocol/https/HttpsURLConnection/CloseKeepAliveCached.java
line 47:
> 45: * @library /test/lib
> 46: *
> 47: * @run main/othervm -Dtest.separateThreads=false CloseKeepAliveCached
> false
The `test.separateThreads` property is used to statically set
`separateServerThread` on line 88, but then on line 230, you set
`separateServerThread` based on `args[0]`. One of those could go away. I think
each `@test` block should only need 1 `@run` instruction, right?
Also, what is being tested differently if the server is in the "sub" thread
versus when it is in the "main" thread?
test/jdk/sun/net/www/protocol/https/HttpsURLConnection/CloseKeepAliveCached.java
line 122:
> 120: * Signal Client, we're ready for his connect.
> 121: */
> 122: serverReady = true;
Change this to a `CountDownLatch`?
test/jdk/sun/net/www/protocol/https/HttpsURLConnection/CloseKeepAliveCached.java
line 203:
> 201: http.disconnect();
> 202: } catch (IOException ioex) {
> 203: if (sslServerSocket != null) {
Is there a reason for closing the server socket in the client thread? If not,
`sslServerSocket` could be made local to `doServerSide()`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23469#discussion_r2661448695
PR Review Comment: https://git.openjdk.org/jdk/pull/23469#discussion_r2661405377
PR Review Comment: https://git.openjdk.org/jdk/pull/23469#discussion_r2661399349
