On Thu, 29 Jan 2026 17:46:49 GMT, Volkan Yazici <[email protected]> wrote:
>> Daniel Jeliński has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Send stop_sending if the InputStream is closed
>> - Close the stream atomically
>
> test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http3/Http3ServerStreamImpl.java
> line 368:
>
>> 366: @Override
>> 367: public void close() throws IOException {
>> 368: if (closeReason.getAndSet(Boolean.TRUE) == Boolean.TRUE)
>> return;
>
> Shouldn't this be `!compareAndSet(null, TRUE)`? Otherwise we allow changing
> the state from `FAILED` to `CLOSED`, which, AFAICT, is not intended.
If it fails, and then you call close(), then it should be closed, regardless on
whether it failed before?
> test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http3/Http3ServerStreamImpl.java
> line 371:
>
>> 369: if (debug.on())
>> 370: debug.log("Closing request body input stream");
>> 371: requestBodyQueue.add(QuicStreamReader.EOF);
>
> Do we need `requestBodyQueue.add(QuicStreamReader.EOF)` here and in
> `resetStream`, given `read()` will propagate the non-null `closeReason`
> thrown by `current()` anyway, and `current()` is the only reader of
> `requestBodyQueue`?
Yes. The read() call might be blocked taking from the queue (when the queue is
empty) at the time of reset/close - which can happen asynchronously. This is
the only way to wakeup the reader so that read() gets unblocked and throws.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2742933385
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2742940995
