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

[Daniel Fuchs]

On Wed, 21 May 2025 16:50:28 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.
>
> Mikhail Yankelevich has updated the pull request incrementally with one additional commit since the last revision:
> 
>   cleanup

src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 228:

> 226: 
> 227:     public final boolean isFinishing() {
> 228:         return finishedLatch.getCount()==0;

Suggestion:

        return finishedLatch.getCount() == 0;

src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 238:

> 236:      * but still stops as soon as maximum delay is reached.
> 237:      *
> 238:      * @param delay maximum delay in stopping the server

Suggestion:

     * This ensures that the server is stopped immediately after all exchanges are complete. HttpConnections will be forcefully closed if active exchanges do not
     * complete within the imparted delay.
     *
     * @param delay maximum delay to wait for exchanges completion, in seconds

src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 489:

> 487: 
> 488:         public void run() {
> 489:              while (!isFinishing()) {

The name of the method is not great - but since it's pre-existing I'm OK with that.

Suggestion:

            // isFinishing() will be true when there are no active exchange after terminating
             while (!isFinishing()) {

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

> 162:         // Complete the exchange 10 second into the future.
> 163:         // Runs in parallel, so won't block the server stop
> 164:         final Duration exchangeDuration = Duration.ofSeconds(Utils.adjustTimeout(5));

Let's use 10 since we're speaking about 10s everywhere. It doesn't really matter since we should finish way before 10 (unless the bug is present).

Suggestion:

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

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2102283879
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2102290292
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2102302993
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2102312019