On Mon, 13 Jan 2025 16:07:45 GMT, Daniel Fuchs <[email protected]> wrote:
> There are a few places in the HttpClient code base where we close the
> connection when some error/exception happens. This can some time trigger a
> race condition where closing the connection in turns causes a "connection
> closed locally" exception to get reported instead of the original exception.
> This typically happens if another thread is attempting to read from / write
> to the connection concurrently when the original exception that caused the
> connection to get closed occurred.
>
> The fix makes sure that we store the first exception in the underlying
> connection before closing it, and that this is the exception that gets
> subsequently reported.
>
> Some minor drive by test fixes. No new regression test.
src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java line
334:
> 332: if (cf.isCompletedExceptionally()) {
> 333: // if an error occurs during subscription
> 334: connection.close(cf.exceptionNow());
`Future.exceptionNow()` (among other things) states that it will throw a
`IllegalStateException` if the Future was cancelled. We do have a check for
`cf.isCompletedExceptionally()` before invoking this method. However, the
`CompletableFuture.isCompletedExceptionally()` states that it returns `true`
even in the case of cancellation:
> Returns {@code true} if this CompletableFuture completed exceptionally, in
> any way. Possible causes include cancellation ...
So I suspect this call to `exceptionNow()` might need a rethink or some
additional checks?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23080#discussion_r1914539627
