On Mon, 22 Feb 2021 16:44:15 GMT, Daniel Fuchs <[email protected]> wrote:
>> Hi,
>>
>> Please find here a fix for:
>> 8262027: Improve how HttpConnection detects a closed channel when
>> taking/returning a connection to the pool
>>
>> While writing a new test to verify that it was possible to handle proxy
>> *and* server authentication manually when both proxy and server required
>> authentication, I stumbled on a race condition where the next request after
>> receiving 407 would manage to retrieve the connection from the pool just
>> before the connection close was receive from the proxy. Since the test was
>> using POST, and POST are not retried by default, this caused the test to
>> fail randomly and intermittently.
>>
>> This fix proposes to add a checkOpen() method to HttpConnection, which we
>> can call just after retrieving a connection from the pool. This method will
>> attempt to read 1 byte from the channel. Because the connection was in the
>> pool, it should not have received anything, and because the channel is
>> non-blocking, the `read` should always return 0, unless the channel has been
>> closed. This forces an early check on the channel state, rather then waiting
>> for the selector to wake up the Selector Manager Thread - which might happen
>> too late.
>>
>> This is not a 100% silver bullet, but it drastically reduced the number of
>> failures I was observing (to 0 after several 100 loops of testing on all
>> machines). The only other failures I observed was on windows, where
>> apparently closing the socket on the server side can cause a reset, even
>> when SO_LINGER and TCP_NODELAY are specified. I solved that by adding a
>> small delay between socket.shutdownOutput() and socket.close() in the test
>> proxy - when running on windows.
>
> Daniel Fuchs has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains six additional
> commits since the last revision:
>
> - Reworked ProxyServer to reuse the connection in case it can detect that
> there will be no request body
> - Merge branch 'master' into proxy-server-auth-8262027
> - Added blank line
> - Fixed trailing white spaces
> - Remove commented code in test
> - 8262027: Improve how HttpConnection detects a closed channel when
> taking/returning a connection to the pool
Marked as reviewed by chegar (Reviewer).
src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java line
154:
> 152:
> 153: /**
> 154: * Forces a call to the native implementation of the connection
"of the connection's channel" ?
src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java line
189:
> 187: close();
> 188: } catch (IOException x) {
> 189: return false;
maybe it's not worth it, but you could optionally log this situation?
-------------
PR: https://git.openjdk.java.net/jdk/pull/2649
