RFR: 8304065: HttpServer.stop should terminate immediately if no exchanges are in progress
Daniel Fuchs
dfuchs at openjdk.org
Mon Apr 7 14:04:08 UTC 2025
On Sat, 5 Apr 2025 10:23:33 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:
> Please help review this PR which improves `HttpServer::stop` termination timing to better fit user expectations.
>
> This PR:
>
> * Makes `ServerImpl::stop` continue without delay when there are no active exchanges during shutdown
> * Attempts to interrupt the dispatcher thread before joining it, giving long-living interruptible exchanges a chance to finish early
> * Adds testing for boundary conditions with or without an active exchange, delay occurring before exhange completes and exchange completing before delay.
>
> Additionally, `ServerImpl::stop` is updated to use `System::nanotime` instead of `System::currentTimeMillis` when calculating wait times.
>
> The test relies on timing to assert the order of events. Not perfect, but it seems to work.
>
> (This part of the code base is rather new to me. A bit of hand-holding should be expected when reviewing this PR)
src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 238:
> 236: selector.wakeup();
> 237: long latest = System.nanoTime() + Duration.ofSeconds(delay).toNanos();
> 238: while (activeExchanges() > 0 && System.nanoTime() < latest) {
I don't think that works unfortunately. We need something that takes into account that an exchange may have been started but hasn't reached the point where startExchange has been called.
The problem is that exchangeCount is incremeted asynchronously in the ExchangeImpl constructor, and the ExchangeImpl is created by the Exchange::run method - which is run asynchronoously in the executor (submitted by the dispacher thread).
Possibly posting a StopRequested event to the dispatcher thread, and have the dispacther thread switch finished = true when is sees the StopRequested event and notices that allConnections only contains idleConnections (in which case we could also assert that exchangeCount == 0).
Unless I'm not mistaken this is not going to be a trivial fix.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24467#discussion_r2031318272
More information about the net-dev
mailing list