Code Review Request (7161503 subcase) 7142596: RMI JPRT tests are failing

[Olivier Lagneau]

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??!? 
>> (Sorry.)
>>
>> 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 
> cleanup.
>
> 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.

Olivier.
>
> Thanks,
> Olivier.
>