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

[Martin Buchholz]

Latest webrev looks good to me.


On Wed, Mar 12, 2014 at 1:36 AM, Ivan Gerasimov
<ivan.gerasimov at oracle.com>wrote:

>  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> 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/
>>
>> 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> 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/
>>>
>>> Sincerely yours,
>>> Ivan
>>>
>>
>>
>>
>
>