RFR: 8304065: HttpServer.stop should terminate immediately if no exchanges are in progress [v4]
Michael McMahon
michaelm at openjdk.org
Tue May 27 13:39:52 UTC 2025
On Fri, 23 May 2025 08:48:32 GMT, Mark Sheppard <msheppar at openjdk.org> wrote:
> If I have read the original code correctly there is a flaw in it, that is the startExchange is never called. Thus the code in handleEvent won’t executed as anticipated
>
> 391 int exchanges = endExchange(); 392 if (terminating && exchanges == 0) { 393 finished = true; 394 }
>
> Exchanges will always be negative. This is inherited in the new update
>
> WRT current PR, I don't think it is good that the state of the server is encapsulated in the state of the CountDownLatch. The use of the CountDownLatch to synchronise the stop and the Dispatcher shutdown is fine, but having the server state encapsulated in the CountDownLatch value is a bit oblique or obfuscating. I think the original volatile boolean variable is a clearer semantics
>
> I would make stop synchronised to have only one thread executing at time. First check on entry then becomes test on the server's current state. This means that this state management needs some attention.
>
> The addition of Stop event is a neat idea, but draws attention to the semantics of the stop. What is the desired semantics of stop, to not accept any new connections, i.e. stop the listener immediately, not accept new requests on existing connections. To allow existing Exchanges to complete and then shutdown all extant connections.
startExchange() is called in the constructor for ExchangeImpl, which is not changing in this PR.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/25333#issuecomment-2912550272
More information about the net-dev
mailing list