On Fri, 9 May 2025 14:39:53 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> Hi, >> >> Please find here a PR for the implementation of [JEP 517: HTTP/3 for the >> HTTP Client API](https://openjdk.org/jeps/517). >> >> The CSR can be viewed at [JDK-8350588: Implement JEP 517: HTTP/3 for the >> HTTP Client API](https://bugs.openjdk.org/browse/JDK-8350588) >> >> This JEP proposes to enhance the HttpClient implementation to support HTTP/3. >> It adds a non-exposed / non-exported internal implementation of the QUIC >> protocol based on DatagramChannel and the SunJSSE SSLContext provider. > > Daniel Fuchs has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 422 commits: > > - merge latest changes from master branch > - Undo whitespace change > - Remove unnecessary import > - Merge remote-tracking branch 'origin/master' into http3 > - Fix test license > - Remove leftover file (test was moved to parent directory) > - Remove unnecessary import > - Update throws clauses > - Merge remote-tracking branch 'origin/master' into http3 > - 8354275: Add HTTP/3 tests to `EmptyAuthenticate` > - ... and 412 more: https://git.openjdk.org/jdk/compare/568dcc15...8c27f53c I spotted some inacuracy in QuicEndpoint. I will take care of these. src/java.net.http/share/classes/jdk/internal/net/http/Http3ClientImpl.java line 158: > 156: quicIdleMillis = h3Millis + 60000; // a minute more > than h3 timeout > 157: } > 158: transportParameters.setIntParameter(max_idle_timeout, > quicIdleMillis); I understand we don't want the QUIC Idle Timeout to kill idle connections before the HTTP/3 idle timeout has expired. But on the other hand, we should not let the HTTP/3 idle timeout set a quic idle timeout that is less than the default quic timeout. We should take into account the value of the default quic timeout that we use in QuicConnectionImpl here. src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicEndpoint.java line 737: > 735: // channel, and a single selector thread, so we can do > 736: // the reading directly in the selector thread and offload > 737: // the parsing (the readLoop) to the executor. This comment is outdated. We actually stop reading from the channel when the amount of data buffered exceeds a high watermark threshold. src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicEndpoint.java line 822: > 820: if (this.buffered.get() >= MAX_BUFFERED_HIGH) { > 821: pauseReading(); > 822: readLoopScheduler.runOrSchedule(executor); This line should not be needed: we should already have kicked the read loop at line 797. ------------- PR Review: https://git.openjdk.org/jdk/pull/24751#pullrequestreview-2846207620 PR Review Comment: https://git.openjdk.org/jdk/pull/24751#discussion_r2092779174 PR Review Comment: https://git.openjdk.org/jdk/pull/24751#discussion_r2092786818 PR Review Comment: https://git.openjdk.org/jdk/pull/24751#discussion_r2092805633
