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

Hello Darragh,

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

I ran some experiments with this test and it appears that if the handler isn't 
invoked then we won't reach the place where we assert the `handlerIsDaemon` 
field. So I think the current form is OK (of course, the PR needs a couple of 
updates suggested by Daniel). 

Just as an extra precaution, what you could perhaps do is initialize 
`handlerIsDaemon` to `true` instead of the current `false`. That way it will 
only be set to `false` (the expected value) only when the handler is invoked in 
a non daemon thread.

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

PR Comment: https://git.openjdk.org/jdk/pull/18514#issuecomment-2036858656

Reply via email to