RFR: 7152183: TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently [sol]

[Martin Buchholz]

Hi all,

Yeah, this particular test is rather racy - sorry about that.
We need to call p.destroy when the other thread is in the middle of a
read() system call, and there's no way to know for sure - seeing java
"read" in the stacktrace is not enough, since it may not have gotten to the
system call yet.

suggestions:
pull the computation of the inputstream before the latch to narrow the
window a bit:

final InputStream s;
switch (action & 0x1) {
case 0: s = p.getInputStream(); break;
case 1: s = p.getErrorStream(); break;
default: throw
}
latch.countdown();
switch (action & 0x2) {
case 0: r = s.read(); break;
case 1: r = s.read(bytes); break;
}

Examining the stack trace to look for "read" is clever but does not
actually eliminate the race.

Looking in UNIXProcess.java.solaris I see the use
of DeferredCloseInputStream.  We can eliminate the race on solaris (i.e. if
the inputstream.getclass isDeferredCloseInputStream)  by looping until the
useCount field of the DeferredCloseInputStream is > 0, using ugly but
effective reflective code.  This should allow us to avoid the horrible
sleep for 200ms.

You should use yield instead of sleep between loop iterations while waiting
for the useCount to be bumped.

On other platforms this is not an issue, I think.

Martin

On Thu, Oct 4, 2012 at 12:39 AM, Alan Bateman <Alan.Bateman at oracle.com>wrote:

> On 03/10/2012 22:44, Rob McKenna wrote:
>
>> Hi folks,
>>
>> The only way I can see this test failing in this manner[*] is if we
>> destroy the process before we begin the read. That being the case I've
>> jacked up the sleep (giving the reader thread a little more time to get
>> cracking) and added a check to see if the threads stack has entered a read
>> call.
>>
>> http://cr.openjdk.java.net/~**robm/7152183/webrev.01/<http://cr.openjdk.java.net/~robm/7152183/webrev.01/><
>> http://cr.openjdk.java.net/%**7Erobm/7152183/webrev.01/<http://cr.openjdk.java.net/%7Erobm/7152183/webrev.01/>
>> >
>>
>> Feedback greatly appreciated.
>>
>>     -Rob
>>
>>
>> [*] le trace:
>>
>>  So stack traces are masculine, I didn't know that.
>
> I think your analysis is right, it's just that the sleep(10) is not
> sufficient to ensure that the thread gets to the read method. Increasing
> the sleep is probably sufficient. The hack to look at the stack trace makes
> it more robust for really extreme cases, at the cost of potential further
> maintenance in the event that the implementation changes. In any case it's
> good to resolve this intermittent test failure.
>
> -Alan
>