RFR [8011537]: (fs) Path.register(..) clears interrupt status of thread with no InterruptedException
Hello! The interrupted status of a thread gets cleared with a call to Path.register(). This is due to call to Object.wait(). Below is the fix with an updated regtest. Would you please help review it? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8011537 WEBREV: http://cr.openjdk.java.net/~igerasim/8011537/0/webrev/ Sincerely yours, Ivan
On 06/05/2014 14:56, Ivan Gerasimov wrote:
Hello!
The interrupted status of a thread gets cleared with a call to Path.register(). This is due to call to Object.wait().
Below is the fix with an updated regtest. Would you please help review it?
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8011537 WEBREV: http://cr.openjdk.java.net/~igerasim/8011537/0/webrev/ The change looks good, an oversight in the original implementation.
A minor comment on the test is that we can using try-with-resources (as WatchService is Closeable). The register could also use the varargs variant to avoid creating the array explicitly but either way is fine. -Alan
Thank you Alan!
The interrupted status of a thread gets cleared with a call to Path.register(). This is due to call to Object.wait().
Below is the fix with an updated regtest. Would you please help review it?
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8011537 WEBREV: http://cr.openjdk.java.net/~igerasim/8011537/0/webrev/
The change looks good, an oversight in the original implementation.
A minor comment on the test is that we can using try-with-resources (as WatchService is Closeable). The register could also use the varargs variant to avoid creating the array explicitly but either way is fine.
I tried to make the added test consistent with the rest of the code. I can surely rewrite it, but should I refactor other code as well then? Sincerely yours, Ivan
On 06/05/2014 15:52, Ivan Gerasimov wrote:
I tried to make the added test consistent with the rest of the code. I can surely rewrite it, but should I refactor other code as well then? I think the the existing tests pre-date the vararg variant of register, there isn't a big need to change those.
For the new test then it would be good to use try-with-resources here. It would be good to change testExceptions too but not important for this change. -Alan.
Thank you Alan! On 06.05.2014 19:02, Alan Bateman wrote:
On 06/05/2014 15:52, Ivan Gerasimov wrote:
I tried to make the added test consistent with the rest of the code. I can surely rewrite it, but should I refactor other code as well then? I think the the existing tests pre-date the vararg variant of register, there isn't a big need to change those.
For the new test then it would be good to use try-with-resources here.
Okay, changed it.
It would be good to change testExceptions too but not important for this change.
I think it can be changed with the upcoming fix for 8042470, which will include modifications to this test anyways. Here's the updated webrev: http://cr.openjdk.java.net/~igerasim/8011537/1/webrev/ Sincerely yours, Ivan
participants (2)
-
Alan Bateman
-
Ivan Gerasimov