[9] RFR: 8164159: java/nio/file/WatchService/UpdateInterference.java test leaves daemon threads

[Artem Smotrakov]

Thank you for review Alan.

Please see inline.


On 08/18/2016 12:42 PM, Alan Bateman wrote:
>
>
> On 18/08/2016 18:19, Artem Smotrakov wrote:
>>
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8164159
>>> Webrev: http://cr.openjdk.java.net/~asmotrak/8164159/webrev.00/
>
> The comment in the finally block is confusing. It says that threads 
> will stop before the WatchService is closed but it's the other way 
> around - the WatchService::close method will be invoked before the 
> finally block executes. This means that ClosedWatchServiceException 
> will likely br thrown and the stack trace recorded in the test output.
There are two try-finally blocks. First one creates a WatchService, and 
contains a nested try-finally block. The "stop" flag is set to true in 
this nested try-finally block, so that the test waits for threads to 
finish before the watch service is closed.

I am not a fan of many nested "try" blocks, anonymous classes, etc, 
because they don't usually make code more readable. But on the other 
hand, I didn't want to re-write the test much.
>
> A minor suggestion, can Thread one and two be renamed to t1 and t2 to 
> make it a bit more readable?
Sure. I am wondering if we could use some other names, for example, 
mickeyMouseThread and winnieThePoohThread. Anyway, t1 and t2 are also fine.
> Also no need to initialize to volatile stop to false as that is its 
> initial value.
Of course.

Could you please take a look at updated webrev?

http://cr.openjdk.java.net/~asmotrak/8164159/webrev.01/

Artem
>
> -Alan.