RFR: 8304065: HttpServer.stop should terminate immediately if no exchanges are in progress

[Daniel Fuchs]

On Tue, 20 May 2025 15:51:50 GMT, Mikhail Yankelevich <myankelevich at openjdk.org> wrote:

> HttpServer::stop will terminate the server immidiately after all exhcnages are complete.
> If the exchanges take longer then the specified delay it will terminate straight after the delay, the same as the previous behaviour.
> 
> Used to wait until the delay is complete at all times, regardless of the number of active exchanges.
> 
> Tests based on @eirbjo work, so adding Eirik as a contributor.

test/jdk/com/sun/net/httpserver/ServerStopTerminationTest.java line 128:

> 126: 
> 127:         // Time the shutdown sequence
> 128:         final Duration delayDuration = Duration.ofSeconds(2);

We should use a greater margin here since we're not expecting to wait for that duration, and probably use `Utils.adjustTimeout()` too. I'd suggest something like:

Suggestion:

        final Duration delayDuration = Duration.ofSeconds(Utils.adjustTimeout(5));

test/jdk/com/sun/net/httpserver/ServerStopTerminationTest.java line 160:

> 158:         // Complete the exchange 10 second into the future.
> 159:         // Runs in parallel, so won't block the server stop
> 160:         final Duration exchangeDuration = Duration.ofSeconds(10);

Suggestion:

        final Duration exchangeDuration = Duration.ofSeconds(Utils.adjustTimeout(5));

test/jdk/com/sun/net/httpserver/ServerStopTerminationTest.java line 190:

> 188:     public void noActiveExchanges() {
> 189:         // With no active exchanges, shutdown should complete immediately
> 190:         final Duration delayDuration = Duration.ofSeconds(1);

Suggestion:

        final Duration delayDuration = Duration.ofSeconds(Utils.adjustTimeout(5));

test/jdk/com/sun/net/httpserver/ServerStopTerminationTest.java line 207:

> 205:         timeShutdown(delayDuration);
> 206:         log("Shutting down the server the second time");
> 207:         timeShutdown(delayDuration);

Should we check the time it took the server to shutdown here too?

test/jdk/com/sun/net/httpserver/ServerStopTerminationTest.java line 228:

> 226:                 .uri(URI.create("http://"
> 227:                                 + server.getAddress().getAddress().getHostAddress()
> 228:                                 + ":" + server.getAddress().getPort() + "/"))

We should use `URIBuilder` here to take care of IPv4 vs IPv6 etc...

test/jdk/com/sun/net/httpserver/ServerStopTerminationTest.java line 238:

> 236:                     // count the latch down to allow the handler to complete
> 237:                     // and the server's dispatcher thread to proceed; The handler
> 238:                     // is called withing the dispatcher thread since we haven't

Suggestion:

                    // is called within the dispatcher thread since we haven't

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2098475385
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2098478903
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2098481789
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2098485308
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2098488908
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2098489836