On Tue, 2 Apr 2024 11:55:12 GMT, Darragh Clarke <[email protected]> wrote:
>> Currently this test occasionally doesn't cleanup between runs, sometimes not
>> stopping the server or leaving Streams open
>>
>> Changes:
>> - Use try-with-resources to ensure streams close.
>> - Use try-finally to make sure the server stops before the test exits.
>>
>> I ran tiers 1-3 and ran this specific test on repeat and everything seems
>> stable after the changes
>
> Darragh Clarke has updated the pull request incrementally with one additional
> commit since the last revision:
>
> implemented feedback
> Darragh, one other thing - I haven't paid close attention to this test, so
> can't say for certain - are we sure that when the test code reaches the place
> where we assert on `handlerIsDaemon` field, the `MyHandler.handle()` was
> indeed invoked? A quick look at that test didn't convince me that this code:
>
> ```java
> InputStream is = url.openConnection(Proxy.NO_PROXY).getInputStream();
> read (is);
> ```
>
> is enough to guarantee that the handler was invoked.
>
> I would have looked at the test code and the HttpURLConnection implementation
> myself, but I need to head out now.
That's a good point, from my testing it seems to consistently be reached but
I'm not sure if it's 100% guaranteed. Would a `CountDownLatch` be useful here
maybe, something like this?
} finally {
handlerIsDaemon = Thread.currentThread().isDaemon();
latch.countDown();
}
```
-------------
PR Comment: https://git.openjdk.org/jdk/pull/18514#issuecomment-2032446404
