On Tue, 6 Sep 2022 13:56:15 GMT, Conor Cleary <[email protected]> wrote:
> **Issue**
> When using HTTP/2 with the HttpClient, it can often be necessary to close an
> idle Http2 Connection before a server sends a GOAWAY frame. For example, a
> server or cloud based tool could close a TCP connection silently when it is
> idle for too long resulting in ConnectionResetException being thrown by the
> HttpClient.
>
> **Proposed Solution**
> A new system property, `jdk.httpclient.idleConnectionTimeout`, was added and
> is used to specify in Milliseconds how long an idle connection (idle
> connections are those which have no currently active streams) for the
> HttpClient before the connection is closed.
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java line
1525:
> 1523: }
> 1524:
> 1525: public Optional<Duration> idleConnectionTimeout() {
Perhaps change this to a package private method instead of a public one?
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java line
1695:
> 1693: private Duration getIdleConnectionTimeoutProp() {
> 1694: // Http 2 in prop name somewhere
> 1695: String s =
> Utils.getNetProperty("jdk.httpclient.idleConnectionTimeout");
Is this new system property expected/allowed to be configured even in
`net.properties` file? The implementation of `Utils.getNetProperty` will check
even the `net.properties` file for this value, so just checking if that's what
we want. If we do want this to be configurable in `net.properties`, should we
updated that file to have a commented section which this property and some
documentation about it?
Furthermore, I think we should read this value just once and cache the
`Duration` as a static member of this class instead of reading this more than
once.
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java line
1697:
> 1695: String s =
> Utils.getNetProperty("jdk.httpclient.idleConnectionTimeout");
> 1696: if (s != null)
> 1697: return Duration.ofMillis(Long.parseLong(s));
Should we catch any `NumberFormatException` here and just default to `null`
duration (i.e. no idle timeout)? Same for negative and `0` value(s)?
-------------
PR: https://git.openjdk.org/jdk/pull/10183
