RFR: 8376479: Http3 test server thread deadlock in ThrowingPublishersInRequest

[Daniel Jeliński]

On Tue, 27 Jan 2026 15:41:56 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> This fixes a deadlock between the thread that reads from the RequestBodyInputStream and the thread that tries to close it in response to a stream reset. See the linked JBS ticket for details.
>> 
>> Tier1 and tier2 tests continue to pass. I verified that with this change there are no busy threads at the end of the test.
>
> test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http3/Http3ServerStreamImpl.java line 320:
> 
>> 318:                     close(io);
>> 319:                     var error = this.error;
>> 320:                     if (error != null) throw error;
> 
> we should use the lock around these two lines.

I'll revisit that.

> test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http3/Http3ServerStreamImpl.java line 329:
> 
>> 327:             if (closed) {
>> 328:                 throw new IOException("Stream is closed");
>> 329:             }
> 
> No sure we want that. If there is some unread data we want to read it first.

The InputStream spec states that `read` throws `IOException` if the stream was closed. Note that we no longer set `closed = true` on error.

> test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http3/Http3ServerStreamImpl.java line 334:
> 
>> 332:                 if (closed) {
>> 333:                     throw new IOException("Stream is closed");
>> 334:                 }
> 
> we should not do that if this.error != null.

I'd say that `closed` has a higher priority than `error`, because `closed` means that the stream was closed by the user.

> test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http3/Http3ServerStreamImpl.java line 358:
> 
>> 356:                         var error = this.error;
>> 357:                         if (error == null) return -1;
>> 358:                         throw error;
> 
> these lines should be protected by the lock.

That's not necessary. If we received an EOF, either the InputStream is closed, or the QUIC stream is finished, or it is reset. The closed case was already handled above, and EOF and reset are mutually exclusive.

> test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http3/Http3ServerStreamImpl.java line 371:
> 
>> 369:         public void close() throws IOException {
>> 370:             if (closed) return;
>> 371:             closed = true;
> 
> these lines should be protected by the lock.

I don't think it's needed. The worst thing that could happen is that we will log twice.

> test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http3/Http3ServerStreamImpl.java line 379:
> 
>> 377:         void close(IOException io) {
>> 378:             if (error != null) return;
>> 379:             error = io;
> 
> close can be asynchronous. We should keep the lock here - or use synchronized.

I'll update the code so that these lines will only be called in response to stream reset, once per stream, in a sequential scheduler.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2732995462
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2732997859
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2733011677
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2733022002
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2733025721
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2733028936