RFR: 8304065: HttpServer.stop should terminate immediately if no exchanges are in progress [v5]
Daniel Fuchs
dfuchs at openjdk.org
Tue May 27 14:07:55 UTC 2025
On Tue, 27 May 2025 13:25:38 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:
>
> consolidated events
src/jdk.httpserver/share/classes/sun/net/httpserver/Event.java line 35:
> 33: this.exchange = t;
> 34: }
> 35:
Can you make the `exchange` field final?
src/jdk.httpserver/share/classes/sun/net/httpserver/Event.java line 48:
> 46:
> 47: /**
> 48: * Event indicating that writing is finished.
Suggestion:
* Event indicating that writing is finished for a given exchange.
src/jdk.httpserver/share/classes/sun/net/httpserver/Event.java line 52:
> 50: public static class WriteFinished extends Event {
> 51: WriteFinished(ExchangeImpl t) {
> 52: super (t);
Suggestion:
super(Objects.requiresNonNull(t));
src/jdk.httpserver/share/classes/sun/net/httpserver/Event.java line 56:
> 54: t.writefinished = true;
> 55: }
> 56: }
These two classes could be marked final. Do they need to be public?
The Event class should be marked abstract and sealed. If we want to consolidate all events in a single file, marking the super class sealed with no permits clause will make it harder to sneak in a new subclass in a different file.
src/jdk.httpserver/share/classes/sun/net/httpserver/FixedLengthOutputStream.java line 97:
> 95: } catch (IOException e) {}
> 96: }
> 97: Event.WriteFinished e = new Event.WriteFinished(t);
Suggestion:
Event e = new Event.WriteFinished(t);
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2109291324
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2109283346
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2109288671
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2109272167
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2109292791
More information about the net-dev
mailing list