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.bate...@oracle.com
<mailto:alan.bate...@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
