On Tue, 27 Jan 2026 13:54:07 GMT, Daniel Jeliński <[email protected]> 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.
Changes requested by dfuchs (Reviewer).
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.
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.
test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http3/Http3ServerStreamImpl.java
line 347:
> 345: if (closed) {
> 346: throw new IOException("Stream is closed");
> 347: }
Same here - we should read off any remaining data first.
test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http3/Http3ServerStreamImpl.java
line 355:
> 353: if (closed) {
> 354: throw new IOException("Stream is closed");
> 355: }
ditto - we should not replace the exception that was passed to close(), if any.
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.
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.
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.
-------------
PR Review: https://git.openjdk.org/jdk/pull/29448#pullrequestreview-3711923100
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2732633330
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2732614669
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2732617261
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2732646645
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2732653698
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2732656061
PR Review Comment: https://git.openjdk.org/jdk/pull/29448#discussion_r2732623273
