Code Review Request 7142596: RMI JPRT tests are failing

[Darryl Mocek]

On Mon 09 Jul 2012 05:02:23 PM PDT, Stuart Marks wrote:
>
> On 7/9/12 11:14 AM, Darryl Mocek wrote:
>>
>>>
>>> *** 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.
>>
>> I don't see a simple solution right now. I suggest we table this
>> issue and
>> re-visit it after the commit. Another option is to not include the
>> fix for Bug
>> #7161503 with this fix until this issue has been addressed.
>
>
> Tabling this discussion until after the commit is OK. It would be good
> to have a comment that indicates that testing the exception string is
> a stopgap until we find a better way to distinguish the exceptions.
I'll add a comment.
>
>>
>>>
>>> *** MultipleRegistries.java
>>>
>>>
>>> Not really a big deal, but the way the second registry is created
>>> seems somewhat roundabout. It's not clear to me why the code can't
>>> just do this:
>>>
>>> Registry registryImpl1 = TestLibrary.createRegistryOnUnusedPort();
>>> Registry registryImpl2 = TestLibrary.createRegistryOnUnusedPort();
>>> int port1 = TestLibrary.getRegistryPort(registryImpl1);
>>> int port2 = TestLibrary.getRegistryPort(registryImpl2);
>>
>>
>> This turned out to be an issue with calling
>> LocateRegitry.createRegistry(0),
>> which occurs in TestLibrary.createRegistryOnUnusedPort(). If
>> LocateRegitry.createRegistry(0) is called within the same VM multiple
>> times,
>> after the first registry is created (and not destroyed), the
>> subsequent calls
>> will fail. See the comment above creating the second registry line:
>>
>> // Need to get a random port for the second registry.
>>
>> However, this really isn't the right solution, so I modified
>> TestLibrary.createRegistryOnUnusedPort to catch ExportException
>> (which is
>> thrown if the above issue occurs), get a random port, and attempt to
>> create a
>> registry on that port. See updated TestLibrary. MultipleRegistries
>> has been
>> changed to what you have above as a result.
>
>
> OK, I'll look at your updates when you publish the revised webrev.
>
> It might be reasonable to consider avoiding
> LocateRegistry.createRegistry(0) if it has this behavior, and instead
> (from within the test library) unconditionally call
> getUnusedRandomPort() and then createRegistry() on that port.
I originally had getUnusedRandomPort/createRegistry(randomPort), but 
Alan felt LocateRegistry.createRegistry(0) is a better choice. 
getUnusedRandomPort creates a socket to ensure the port is available, 
then closes it and returns the port number.  It's possible (though 
unlikely) that another process will grab the port after closing and 
before createRegistry(randomPort) executes. 
LocateRegistry.createRegistry(0) ensures the registry is started on a 
random port.

Darryl
>
> s'marks