8245462: HttpClient send throws InterruptedException when interrupted but does not cancel request

[Michael McMahon]

Hi Daniel,

This looks like good work. A couple of points/questions:

- commented code in RequestPublishers.java can be deleted presumably

- RequestPublishers.IterablePublisher :: computeLength(). What is the 
reason for returning -1
    here instead of the computed length.

- why does Stream.registerStream now take a boolean 
'registerIfCancelled'? In particular why set that to true?

- Do you think the spec change is required normatively? Or could we do 
without it, or just add
    informative text instead?

I'm still looking at the test.

Thanks,

Michael.

On 23/06/2020 18:47, Daniel Fuchs wrote:
> Hi,
>
> Please find below a fix for:
> 8245462: HttpClient send throws InterruptedException when
>          interrupted but does not cancel request
> https://bugs.openjdk.java.net/browse/JDK-8245462
>
> webrev:
> http://cr.openjdk.java.net/~dfuchs/webrev_8245462/webrev.00/index.html
>
> The fix goes beyond fixing the problem described in the issue and
> provides a way for cancelling asynchronous requests too.
>
> Indeed, up to now the HttpClient didn't provide an easy way to
> stop/cancel an ongoing HTTP request.
> Up to now the only possible way of doing that would have been
> to provide your own BodyPublisher/BodyHandler/BodySubsriber so
> that you could stop sending the request body (by invoking onError
> on the HttpClient subscriber) or stop the reception of the response
> body (by cancelling the subsription you were given). However there
> would be no way to stop the request outside of these limits.
> If the request body was already sent, you would have to wait for
> the response headers to have been received before you could
> actually attempt to interrupt the exchange.
> This is a bit of a shame since the HTTP/2 protocol does allow
> for resetting streams if the client is no longer interested
> in receiving a response.
>
> This fix proposes to arrange for the HTTP exchange to be
> stopped/cancelled if `cancel(true)` is invoked on the
> CompletableFuture<> objects the default implementation
> of the HttpClient returns, or on completable futures derived
> from them. This makes it possible for the HttpClient implementation
> to take note of the cancellation request asynchronously and then
> stop the exchange as soon as it can.
>
> The synchronous HttpClient::send also makes use of this new mechanism
> to attempt to stop/cancel the exchange if the thread is interrupted
> while it waits for the completable future to be completed.
>
> best regards,
>
> -- daniel