RFR [8034262] Test java/lang/ProcessBuilder/CloseRace.java fails

[Ivan Gerasimov]

Thanks Martin

On 12.03.2014 2:45, Martin Buchholz wrote:
> Well done!
>
> Not checking for interrupt in the loop is clearly my bug.
>
> dumpAllStacks looks like a very useful utility method (basically what 
> jstack does).  It feels like we're reinventing the wheel here, but I'm 
> not sure.
>

jstack calls the hotspot-specific code to dump the stack traces, but I 
used the jdk-level routine.
The dumpAllStacks() function has already done its job, so I could 
probably just remove it.
Though it may be better to keep it just in case.

> Instead of "break theLoop" I would have simply returned from the method.
>

OK, did that. Here's the updated webrev:

http://cr.openjdk.java.net/~igerasim/8034262/2/webrev/

Sincerely yours,
Ivan

>
> On Tue, Mar 11, 2014 at 11:03 AM, Ivan Gerasimov 
> <ivan.gerasimov at oracle.com <mailto:ivan.gerasimov at oracle.com>> wrote:
>
>     Thank you Martin!
>
>
>     On 11.03.2014 19:04, Martin Buchholz wrote:
>>     Thanks for working on my brittle racy tests.
>>
>>     Adding a latch as you did is a good improvement, although there's
>>     a good chance the real problem lies elsewhere.
>>
>>     My standard name for such latches is "threadsStarted", which I
>>     think is a bit better than "startedSignal".  Please rename.
>>
>     No problem, renamed.
>
>
>>     My simple calls to Thread.join are too optimistic.
>>     More likely to be helpful is code like (pseudocode follows):
>>
>>     thread.join(10, SECONDS);
>>     if (thread.isAlive()) {
>>       dumpAllStacks();
>>       fail();
>>     }
>>
>     Yes, it was a good idea to do that!
>     After implementing your suggestion, I could finally reproduce the
>     failure.
>     The OpenLoop child thread was spinning in the do-while loop,
>     waiting for 'count of fds in use' to become 3.
>     The simple solution is to add a check weather the current thread
>     is interrupted to this and other loops.
>
>     Would you please take a look at the updated webrev?
>     http://cr.openjdk.java.net/~igerasim/8034262/1/webrev/
>     <http://cr.openjdk.java.net/%7Eigerasim/8034262/1/webrev/>
>
>     Sincerely yours,
>     Ivan
>
>
>>     That's  more work to implement - optional.
>>
>>
>>     On Tue, Mar 11, 2014 at 5:24 AM, Ivan Gerasimov
>>     <ivan.gerasimov at oracle.com <mailto:ivan.gerasimov at oracle.com>> wrote:
>>
>>         Hello everybody!
>>
>>         The test java/lang/ProcessBuilder/CloseRace.java was reported
>>         to intermittently fail.
>>         The test timed out, which should mean that at least one of
>>         the child threads was never interrupted.
>>
>>         I couldn't reproduce the failure, but I suspect it might
>>         happen due to call to interrupt() before the child thread
>>         became alive (I'm not really sure if it's possible to be
>>         non-alive after call to start()).
>>         The fix is to explicitly synchronize children with the parent.
>>
>>         Would you please help review the fix?
>>
>>         BUGURL: https://bugs.openjdk.java.net/browse/JDK-8034262
>>         WEBREV:
>>         http://cr.openjdk.java.net/~igerasim/8034262/0/webrev/
>>         <http://cr.openjdk.java.net/%7Eigerasim/8034262/0/webrev/>
>>
>>         Sincerely yours,
>>         Ivan
>>
>>
>
>