On Fri, 25 Nov 2022 11:09:11 GMT, Darragh Clarke <[email protected]> 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
