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

Reply via email to