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

Reply via email to