Question about the bug https://bugs.openjdk.java.net/browse/JDK-8031179

Eric Wang yiming.wang at oracle.com
Fri Jan 24 07:14:07 UTC 2014


Hi Stuart,
Thanks for the suggestion! sorry for reply this mail late as i was busy 
on other tasks
The webrev has been in the internal review process. Based on the 
suggestion, here is a summary of changes:

1. Add othervm options to tests:
java/rmi/Naming/DefaultRegistryPort.java
java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java
java/rmi/Naming/LookupNameWithColon.java
java/rmi/server/UnicastRemoteObject/exportObject/GcDuringExport.java
sun/rmi/rmic/RMIGenerator/RmicDefault.java
java/rmi/MarshalledObject/compare/Compare.java
java/rmi/MarshalledObject/compare/HashCode.java

2. Remove java/rmi and sun/rmi from othervm.dirs of TEST.ROOT

3. Filed a another bug https://bugs.openjdk.java.net/browse/JDK-8032629 
for exclusiveAccess.dirs

Thanks,
Eric

On 2014/1/18 8:45, Stuart Marks wrote:
> Hi Eric,
>
> Thanks for doing the analysis of the tests that need /othervm. The 
> list of tests that don't need /othervm looks good. One subtlety is 
> this one:
>
> java/rmi/activation/ActivationGroupDesc/checkDefaultGroupName/CheckDefaultGroupName.java 
>
>
> Most activation tests do need /othervm, but this one doesn't, since 
> all it does is create an ActivationGroupDesc instance, which has no 
> side effects. This is unusual for the activation APIs, since most of 
> them are quite intertwined with the rest of the activation subsystem. 
> So, for this test, the lack of /othervm warrants a comment explaining 
> why /othervm is unnecessary.
>
> Regarding merging of the tests 
> java/rmi/MarshalledObject/compare/Compare.java and HashCode.java, this 
> is fairly subtle and not obviously wrong, but it's not obviously right 
> either. Note that different jtreg tests -- even in agentvm or samevm 
> mode -- load the test class in a fresh classloader, which means that 
> the statics are reinitialized. This provides some degree of isolation 
> of the tests even when they're reusing the same JVM. By contrast, 
> calling the two different methods from within the same test exposes 
> the second one to initializations left from the first one. In addition 
> (though I'm not too concerned about this) if the first test fails the 
> second won't be run at all. Merging these tests is kind of out of 
> scope for this particular bug, and since I wasn't fully able to 
> convince myself that the merged tests have the same semantics as the 
> unmerged tests, I'd prefer not to see the merging of these tests as 
> part of this changeset.
>
> (Some cleanup of these two tests is probably warranted eventually. I'd 
> take a different approach of merging and refactoring the sources but 
> keeping them as separate test runs, using two @run tags. But we should 
> work on that separately. Meanwhile, the overhead of having these as 
> separate tests is minimal, especially if they're not run in /othervm 
> mode.)
>
> I don't think the removal of java/rmi/Naming from exclusiveAccess.dirs 
> is safe at this point. The DefaultRegistryPort.java test uses the 
> default registry port, not a unique one. Conceptually it would need to 
> be converted to use the TestLibrary unique port stuff, but the test is 
> actually about using the default port. So, the solution here isn't 
> obvious.
>
> In addition, the 
> java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java test also 
> uses the default registry port. It too will need to converted before 
> java/rmi/Naming is removed from exclusiveAccess.dirs.
>
> The java/rmi/Naming/LookupIPv6.java test was converted to use the 
> TestLibrary unique port technique, but only partially. The registry is 
> created on the unique port, but the Naming.lookup() call on the last 
> line of the test doesn't provide a port number, so it does the lookup 
> on the default port instead. This will cause the test to fail in 
> almost all cases.
>
> I have to ask, did you run this test with your modifications?
>
> (Well, the test would pass if IPv6 is not enabled on the machine 
> running the test, but it only passes because the part of the test that 
> creates and uses the registry is bypassed entirely if IPv6 is not 
> enabled. If you're modifying code, you need to take responsibility for 
> ensuring that the code being modified is actually being run and is 
> doing what you expect.)
>
> LookupIPv6.java also needs to have these lines added to the test tags:
>    * @bug 4402708
> + * @library ../testlibrary
> + * @build TestLibrary
>    * @run main/othervm -Djava.net.preferIPv6Addresses=true LookupIPv6
> (Their absence will cause the test also to fail in a clean build, but 
> the test will find a TestLibrary class if one had been compiled by a 
> previous test that had required it.)
>
> Maybe we should separate the othervm changes (removal of the rmi dirs 
> from othervm.dirs, and addition of /othervm) from the 
> exclusiveAccess.dirs changes. A separate bug would be filed for 
> exclusiveAccess.dirs. I know this means the bug count won't go down, 
> but dealing with exclusiveAccess.dirs means that the logic of various 
> tests will need to be change to use the unique port mechanism. This is 
> a fair chunk of work and it's logically distinct from the othervm work.
>
> If we also remove the merging of the MarshalledObject tests, I think 
> we can proceed with the othervm changes.
>
> Eric, can you make these changes and send out another webrev?
>
> Thanks.
>
> s'marks
>
>
>
> On 1/16/14 7:18 AM, Eric Wang wrote:
>> Hi Stuart,
>>
>> I have generated a webrev for review, can you please help to check 
>> whether the changes are OK.
>> http://cr.openjdk.java.net/~ewang/JDK-8031179/webrev.00/ 
>> <http://cr.openjdk.java.net/%7Eewang/JDK-8031179/webrev.00/>
>>
>> Also need your help to confirm that the rest 9 tests in previous mail 
>> don't need the /othervm option indeed.
>>
>> Thanks,
>> Eric
>> On 2014/1/16 22:55, Eric Wang wrote:
>>> Hi Stuart,
>>>
>>> I'm working on the bug 
>>> https://bugs.openjdk.java.net/browse/JDK-8031179 to add /othervm 
>>> option to rmi tests, so they can removed from the item othervm.dir 
>>> and exclusiveAccess.dirs.
>>> By searching the directories java/rmi, sun/rmi and java/rmi/Naming, 
>>> there are 14 tests without othervm option, but only 5 tests need to 
>>> the /othervm tag. they are:
>>>
>>> Tests need /othervm option:
>>> java/rmi/Naming/DefaultRegistryPort.java
>>> java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java
>>> java/rmi/Naming/LookupNameWithColon.java
>>> java/rmi/server/UnicastRemoteObject/exportObject/GcDuringExport.java
>>> sun/rmi/rmic/RMIGenerator/RmicDefault.java
>>>
>>> Tests don't need the /othervm option:
>>> java/rmi/activation/ActivationGroupDesc/checkDefaultGroupName/CheckDefaultGroupName.java 
>>>
>>> java/rmi/MarshalledObject/compare/Compare.java
>>> java/rmi/MarshalledObject/compare/HashCode.java
>>> java/rmi/MarshalledObject/compare/NullReference.java
>>> java/rmi/server/Unmarshal/PrimitiveClasses.java
>>> sun/rmi/log/ReliableLog/LogAlignmentTest.java
>>> sun/rmi/log/ReliableLog/Recovery.java
>>> sun/rmi/log/ReliableLog/SnapshotSize.java
>>> sun/rmi/rmic/classpath/RMICClassPathTest.java
>>>
>>> Also i have a bit confuse about the test Compare.java and 
>>> HashCode.java in java/rmi/MarshalledObject/compare directory, as 
>>> they should be merged into one test so that we don't need to add 
>>> additional /othervm option for 2 test.
>>>
>>> Thanks,
>>> Eric
>>>
>>
>




More information about the core-libs-dev mailing list