On Tue, 27 May 2025 13:25:38 GMT, Mikhail Yankelevich <myankelev...@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