RFR: 8294047: HttpResponseInputStream swallows interrupts [v7]

Daniel Fuchs dfuchs at openjdk.org
Thu Dec 1 15:56:30 UTC 2022


On Thu, 1 Dec 2022 13:36:57 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:
> 
>   set interrupt status on thread before throwing exception

Changes requested by dfuchs (Reviewer).

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

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

I suggest checking that the exception cause is the expected InterruptedException. If I'm not mistaken something like that should do it:


var thrown = assertThrows(IOException.class, () -> response.body().readAllBytes(), "expected IOException");
var cause = thrown.getCause();
assertTrue(cause instanceof InterruptedException, cause + " is not an InterruptedException");

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

> 166:                 assertThrows(IOException.class, () -> response.body().readAllBytes(), "expected IOException");
> 167:             } catch (Exception e) {
> 168:                 throw new RuntimeException(e);

You might want to catch Throwable, then print the stack trace and call Asserts.fail here - because unhandled exceptions in threads might just be swallowed.

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

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


More information about the net-dev mailing list