RFR: 8294047: HttpResponseInputStream swallows interrupts [v2]

[Daniel Fuchs]

On Fri, 25 Nov 2022 11:09:11 GMT, Darragh Clarke <duke at openjdk.org> wrote:

>> Currently if a `HttpResonseInputStream` gets interrupted while reading it will just swallow the exception and continue,
>> 
>> This PR changes it to close the stream and throw an IOException, I added a test to cover this which just uses two threads to read the stream then interrupt it.
>
> Darragh Clarke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   implemented feedback

Changes requested by dfuchs (Reviewer).

src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java line 489:

> 487:                     } catch (IOException ignored) {
> 488:                     }
> 489:                     throw new InterruptedIOException();

Thanks for doing that. Maybe we should keep the caught InterruptedException as the cause of the new InterruptedIOException. I'd suggest to add a new utility method to the `...common.Utils` class for that. I mean - something like:

    InterruptedIOException Utils.toInterruptedIOException(InterruptedException ex);

test/jdk/java/net/httpclient/HttpResponseInputStreamInterruptTest.java line 111:

> 109: 
> 110:         CountDownLatch interruptReadyLatch;
> 111:         CountDownLatch interruptDoneLatch;

should be `final`

test/jdk/java/net/httpclient/HttpResponseInputStreamInterruptTest.java line 167:

> 165:                 // countdown on latch, and assert that an IOException is throw due to the interrupt
> 166:                 interruptReadyLatch.countDown();
> 167:                 assertThrows(InterruptedIOException.class, () -> response.body().readAllBytes(), "excepted IOException");

Maybe change the message while you're at it since we're actually expecting InterruptedIOException

-------------

PR: https://git.openjdk.org/jdk/pull/11323