RFR: 7152183: TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently [sol] (original) (raw)

Rob McKenna rob.mckenna at oracle.com
Sun Oct 7 19:48:16 UTC 2012


Thanks Martin,

I've uploaded a new webrev to:

http://cr.openjdk.java.net/~robm/7152183/webrev.02/ <http://cr.openjdk.java.net/%7Erobm/7152183/webrev.02/>

Let me know if this does the job.

 -Rob

On 04/10/12 18:24, Martin Buchholz wrote:

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_ _<mailto: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/%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



More information about the core-libs-dev mailing list