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

[Daniel Fuchs]

Hi Michael,

On 23/07/2020 18:48, Michael McMahon wrote:
> Hi Daniel,
> 
> This looks like good work. A couple of points/questions:
> 
> - commented code in RequestPublishers.java can be deleted presumably

Yes - good point.

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

While writing my test - I noticed that my Iterable was called twice.
Once for computing the length;
Once for sending the data.

If fetching the data is an expensive operation, then you will
pay that cost twice. So my reasoning here is that since you
provide an Iterable - then it should remain lazy, and not
be iterated twice.

Returning -1 has that effect. It means that Content-Length
will not be sent for HTTP/2 (which is allowed - as the
client MAY send Content-Length - while Content-Length is
not really needed for HTTP/2) and that HTTP/1 will use
Chunk encoding to send the body.

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

In case where the operation is canceled before the streamId
is allocated, then there's no reason to actually register the
stream.

Except in the case where the this is the first stream in an
upgraded connection - because the headers have already been
sent - so the stream already de facto exists.

Also I was unsure of the effect of not registering the
stream for push promises - things are a bit different
for server initiated streams.

The entire idea is to avoid unnecessary background activity
(like e.g. sending windows updates or whatever) after the
operation has been canceled.

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

I followed Alan's suggestions on this.

> 
> I'm still looking at the test.

Thanks for the feedback!

best regards,

-- daniel

> 
> 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
>>