Code Review Request - Bug #6948101 & 7142596: RMI JPRT tests are failing

[Darryl Mocek]

See inline

On Sat 14 Apr 2012 09:12:37 AM PDT, Alan Bateman wrote:
>
> On 14/04/2012 00:45, Darryl Mocek wrote:
>>
>> Hello core-libs. Please review this webrev to fix Bug #6948101 and
>> #7142596. This commit fixes concurrency issues with the RMI tests.
>>
>> - Added TestLibrary.getUnusedRandomPort method. This attempts to
>> return a random port number, which is unused, between the port ranges
>> 1024 and 64000 (inclusive). It will try up to 10 times before giving up.
>> - Changed almost all tests from using hard port numbers to using
>> random port numbers for running the RMI Registry and RMID.
>> - Removed othervm from those tests which don't need it.
>> - Added parameters for tests which spawn a separate VM to pass RMI
>> Registry and RMID ports in cases where needed.
>> - Added PropertyPermission to security policy files where needed.
>> - Removed java/rmi and sun/rmi from tests which cannot be run
>> concurrently.
>> - Removed java/rmi and sun/rmi from tests which must run in othervm
>> mode.
>> - Added java/rmi/Naming to list of tests which cannot be run
>> concurrently.
>>
>> Webrev, can be found here:
>> http://cr.openjdk.java.net/~dmocek/7142596/webrev.00
>
> It's great to see this patch as the RMI tests have been just too slow
> and the last area of the core libraries that can't take advantage of
> multi-core machines. Also this patch should eliminate the cascading
> failures that periodically arise when something goes wrong and a test
> leaves a registry on the default port, causing problems for subsequent
> tests.
>
> I've looked briefly at the changes (not a full review, I assume Stuart
> will do that) and if I understand correctly, then this changes don't
> mean we can run the tests in agentvm mode, right? In that case I don't
> think the test directory can be removed from the othervms.dir lists in
> TEST.ROOT.
Ah, yes, with -agentvm specified, many tests fail.  I'll have to add 
java/rmi and sun/rmi back to the list of tests which must be run in 
othervm mode and I should probably add back othervm to the tests I 
removed it from.  At least with the port conflict fix, we can run the 
tests concurrently and they will run much faster.
>
> I see the logic in TestLibrary to choose a random port and I'm just
> wondered if you considered doing LocaleRegistry.createRegistry(0) so
> that the registry binds to an ephemeral port. It would mean that
> TestLibrary would need to get to the transport endpoint and port but
> it would avoid having to try multiple ports.
I would have like to use LocaleRegistry.createRegistry(0), but I don't 
see a way to get the port number once the registry is created, which is 
required.  I originally tried creating a TestLocaleRegistry which 
extends LocateRegistry, with a getPort() method, but this didn't work 
either.

Darryl
>
> -Alan
>
>
>