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

[Daniel Fuchs]

On Fri, 23 May 2025 09:00:39 GMT, Mark Sheppard <msheppar at openjdk.org> wrote:

>> Mikhail Yankelevich has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Suggestions from cr
>>   
>>   Co-authored-by: Daniel Fuchs <67001856+dfuch at users.noreply.github.com>
>
> src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 239:
> 
>> 237:      *
>> 238:      * @param delay maximum delay to wait for exchanges completion, in seconds
>> 239:      */
> 
> make synchronised to allow only one thread to execute stop at any one time

Not sure that's the best solution. But if the `terminating` flag is already set then it means that another thread has initiated stop, and maybe in this case we should just jump to waiting on the latch - no need to post the event again or close the channel and wakeup the selector again. The part in `stop` that need synchronization already have it, so I don't think declaring stop as synchronized would be such a good idea. In particular it would prevent other threads to call getExchange() / endExchange() so I would be afraid it would prevent active exchanges from proceeding - unless we use wait/notify instead of a countdown latch - but that might be even more difficult to get right.

> src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 241:
> 
>> 239:      */
>> 240:     public void stop (int delay) {
>> 241:         if (delay < 0) {
> 
> with the use of CountDownLatch the negative check becomes superfluous i.e.
> 
> public boolean await(long timeout, TimeUnit unit) throws InterruptedException
> Causes the current thread to wait until the latch has counted down to zero, unless the thread is interrupted, or the specified waiting time elapses.
> 
> . . . 
> If the specified waiting time elapses then the value false is returned. If the time is less than or equal to zero, the method will not wait at all.

It's not something we should change though, as it's part of the API contract in `HttpServer::stop`:
https://docs.oracle.com/en/java/javase/24/docs/api/jdk.httpserver/com/sun/net/httpserver/HttpServer.html#stop(int)

> src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 437:
> 
>> 435: 
>> 436:                     logger.log(Level.TRACE, "Write Finished");
>> 437:                     int exchanges = endExchange();
> 
> startExchange is never called => endExchange can return negative ?

I hope that this is a scenario that cannot happen: to end an exchange you would have to start it first.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2104221149
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2104204781
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2104206439