Code Review Request (7161503 subcase) 7142596: RMI JPRT tests are failing
olivier.lagneau at oracle.com
Tue Jul 10 06:56:53 UTC 2012
Le 10/07/2012 08:49, Olivier Lagneau a écrit :
> Hi Stuart,
> I am in vacation for 2 days, but I think I need reply anyway now.
> First thanks for reviewing and giving your time on this.
> Now in the 7161503 SetChilEnv case:
>> *** SetChildEnv.java
>> The testing of the message string from the IOException causes me
>> great concern. This message is defined all the way over in
>> java.io.PipedInputStream, and while it's not localized, it does seem
>> like a pretty fragile dependency. I mean, changing some exception
>> message in java.io might cause an RMI Activation test to fail??!?
>> Certainly we want to ignore spurious errors, and it sounds from the
>> comment like normal termination of rmid causes these exceptions to be
>> thrown. But I'm wondering if rmid crashes, won't we get the same
>> exception and ignore it, improperly?
>> I don't know what the right thing to do is here. It seems like there
>> ought to be a more definitive way to distinguish between normal
>> termination and pipe closure from an error.
> The fact is that the part of the fix we are discussing here is
> cleaning-up of the test.It is not related to 7142596.
> As as said in other mails, this test should be written differently and
> in addition brings a systematic "Pipe Broken exception"
> each time runwith() is called. I mean in the current JDK state, not
> this fixed one. an ugly thing. That was considered to be ok for a test
> success however.
> I surely have gone too far in that fix, and may not have tried to
> clean it up.
> Btw, none of the other rmi java test fixed here does try to do any
> Thus, and because it is important to remain consistant in bug fixes,
> since this one is for 7142596 only,
> I think we should just revert to the existing code regarding the
> DebugExecWatcher and related exception cleanup fix.
> We must then accept to keep this exception raised each time runwith()
> is called, since this is the current state of the code.
> We should also then include in bug description a note stating the
> problem and how we can fix it.
> Such a fix would then be part of recent 7168267.
> a full cleanup fix of DebugExecWatcher code is simple but involves
> testLibrary framework:
> - DebugExecWatcher is a consumer of redirected output of distant rmid:
> it needs to check that "DebugExec" string is found in that stream;
> - Thus a way of dealing with consumer of redirected output is needed
> for it.
> - We define in testlibrary an OutputConsumer interface with only one
> method processInput() that must do the needed work.
> - By default, no such consumer exist and StreamPipe of testLibrary
> does the work as usual.
> - Otherwise, at rmid creation such a consumer is passed and its
> processInput method will be call from streamPipe as needed.
forgot to say this:
- DebugExecWatcher will then disappear as such and "Pipe broken"
IOException problem as well.
More information about the core-libs-dev