On 1/7/2013 3:01 PM, Stuart Marks wrote:
(Note for other readers: StreamPipe is not part of this webrev; the
source is viewable at [1].)
Hi Mandy,
I don't see any place where the StreamPipe threads are interrupted, so
the silent exit on InterruptedIOException probably doesn't ever get
exercised. This handling does seem a bit odd to me though.
For both IOException and InterruptedIOE the thread exits immediately
(and for IOE a stack trace is printed) but this is never reported to
the caller, so if one of these did occur, it's likely that no one
would ever notice -- unless it caused the calling test to fail. It
would probably be reasonable for a StreamPipe to store an indication
of whether it encountered and exception and have code in
JavaVM.waitFor query it and report it to the caller. This seems like a
pretty rare case though. Maybe something to do in a future cleanup.
Swallowing an exception with no good reason seems a potential cause of
reliability issue. As you think this seems a pretty rare case, I'm
fine if you prefer to do this in a future cleanup. Just wanted to point
this out.
Turns out this bug was pretty easy to reproduce by putting a sleep()
at the right place in the StreamPipe loop. This causes the StreamPipe
always to "lose" the race condition so in the old code the test would
fail every time. With the new join() logic the test always passes,
even with sleep() in place; it just runs more slowly.
That's good. Thanks.
Mandy
Thanks,
s'marks
[1]
http://hg.openjdk.java.net/jdk8/tl/jdk/file/a996b57e5541/test/java/rmi/testlibrary/StreamPipe.java
On 1/4/13 4:38 PM, Mandy Chung wrote:
It is a good cleanup and the change looks okay to me. I'll count on
your
testing to verify if this fixes the intermittent problem :)
StreamPipe.run() catches InterruptedIOException and IOException and
ignores the
error. Is it safe? Are the tests expected to fail in some other way
(missing
output?). Should it (or waitFor method) throw a RuntimeException
instead if
any such exception is thrown?
Just curious - Is this intermittent test failure easy to reproduce?
Thanks
Mandy
On 1/3/2013 6:39 PM, Stuart Marks wrote:
Hi all,
Please review these additional RMI test cleanups:
http://cr.openjdk.java.net/~smarks/reviews/7187882/webrev.0/
in service of fixing the following bug:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7187882
The basic problem here is that tests were waiting for the JVM
subprocess to
exit, but not waiting for all output from the subprocess to be
collected. The
fix includes a bit of new infrastructure in RMI's test library and
adjustment
of several other tests to use it.
Thanks,
s'marks
