2nd round code review request: 7055363: jdk_security3 cleanup

[Weijun Wang]

I'll wait then. Plus I can try running the final result.

-Max

On 08/04/2011 06:01 PM, Xuelei Fan wrote:
> On 8/4/2011 5:58 PM, Weijun Wang wrote:
>>
>>
>> On 08/04/2011 05:50 PM, Xuelei Fan wrote:
>>> Good point. I will update the test files. Even we have to make update to
>>> JSSE tests, I would evaluate every tests, and won't add "othervm" tag to
>>> those samevm safe tests. 170+ files, need some more time.
>>
>> So you will add the tag? How long will you use?
> Should be able to finish the update in the end of next week. I have
> updated 20+ files.
>
> Xuelei
>
>> If less than a week, I
>> can wait after you're finished and update the Makefile to call
>> RunasSamevmBatch. Otherwise, I'll push my changeset first and keep
>> RunasOthervmBatch, and in your changeset you change it to RunasSamevmBatch.
>>
>> Thanks
>> Max
>>
>>>
>>>>> ... at least I know tests inside test/closed/
>>>>> sun/security/ssl/java/security/KeyStore can run
>>>>> in samevm. Can you give a whitelist?
>>>>>
>>> The tests won't try to establish a TLS/SSL connection, so they are safe.
>>>
>>> Thanks,
>>> Xuelei
>>>
>>> On 8/4/2011 5:30 PM, Kelly O'Hair wrote:
>>>>
>>>> On Aug 4, 2011, at 11:06 AM, Weijun Wang wrote:
>>>>
>>>>>> Max, if we can get an agree, would you please update the Makefile to
>>>>>> run JSSE in othervm mode  in your fix? So that we don't have to fill
>>>>>> a new CR.
>>>>>
>>>>> There are two ways to do this:
>>>>>
>>>>> 1. Add "@run main/othervm" to all JSSE tests. This means a lot of
>>>>> code changes, but I can automate it.
>>>>>
>>>>> 2. Create new test set for JSSE. This is what I had done before in
>>>>>
>>>>>     http://cr.openjdk.java.net/~weijun/7055363/webrev.00/
>>>>>
>>>>> many changes to Makefile, jprt.properties and jprt.properties in the
>>>>> parent repo.
>>>>>
>>>>> I'd choose #1. What's your opinion?
>>>>
>>>> Just a comment from the peanut gallery... ;^)
>>>>
>>>> I think #1 is the way to go.
>>>>
>>>> One thing we wanted people to be able to do is just cd to the test
>>>> directory and run 'jtreg -samevm' or 'jtreg -agentvm'
>>>> and by changing the test itself, this can work. If at some point
>>>> later someone fixes the test or is able to
>>>> make it work with samevm, they can remove the main/othervm.
>>>> If you do it with the Makefile, you only fixed it for when people use
>>>> the Makefile or with JPRT.
>>>>
>>>> -kto
>>>>
>>>>>
>>>>> BTW, you mentioned HTTPS and JNDI codes. If they are not inside
>>>>> sun/security, I won't touch them this time. Also, at least I know
>>>>> tests inside test/closed/sun/security/ssl/java/security/KeyStore can
>>>>> run in samevm. Can you give a whitelist?
>>>>>
>>>>> Thanks
>>>>> Max
>>>>>
>>>>>
>>>>> On 08/04/2011 12:05 PM, Xuelei Fan wrote:
>>>>>>> - JSSE tests, Xuelei is working on them
>>>>>> It may be a bad news for the purpose to quicken the testing run
>>>>>> time. Most of the JSSE test may not be able to under samevm/agentvm
>>>>>> mode.
>>>>>>
>>>>>> 1. JSSE does not support dynamic system properties For performance
>>>>>> tuning and synchronization, JSSE does not support dynamic system
>>>>>> properties of: "java.protocol.handler.pkgs" "javax.net.ssl.keyStore"
>>>>>> "javax.net.ssl.keyStorePassword" "javax.net.ssl.trustStore"
>>>>>> "javax.net.ssl.trustStorePassword"
>>>>>>
>>>>>> The JSSE default implementation will use these properties when
>>>>>> initializing the static objects, and then any changes to these
>>>>>> properties will be ignored in the same VM.
>>>>>>
>>>>>> There are around 90 of all 170+ JSSE regression tests depends on
>>>>>> system properties.
>>>>>>
>>>>>> 2. JSSE caches TLS sessions, as will result in unexpected reuse of
>>>>>> TLS sessions in samevm/agentvm mode. There are a lot of tests using
>>>>>> the default SSL/TLS context, which will the same TLS session cache.
>>>>>>
>>>>>> There are only a very very few tests could be updated to run in
>>>>>> samevm mode. Because samevm-safe in JSSE tests will result in
>>>>>> unnecessary complex, and the performance improvement is very little,
>>>>>> I would suggest tun all JSSE tests at othervm mode.
>>>>>>
>>>>>> What do you think?
>>>>>>
>>>>>> Max, if we can get an agree, would you please update the Makefile to
>>>>>> run JSSE in othervm mode  in your fix? So that we don't have to fill
>>>>>> a new CR.
>>>>>>
>>>>>> Thanks, Xuelei
>>>>>>
>>>>>> On 8/2/2011 3:02 PM, Weijun Wang wrote:
>>>>>>> Hi All
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~weijun/7055363/webrev.01/
>>>>>>>
>>>>>>> *Attention*: please note that the jtreg mode for jdk_security3 is
>>>>>>> still RunOthervmBatch. Xuelei is now working on the JSSE tests.
>>>>>>> Once he is ready, he will change the mode to RunSamevmBatch.
>>>>>>>
>>>>>>> The code changes:
>>>>>>>
>>>>>>> * Still included in test/ProblemList.txt
>>>>>>>
>>>>>>> - JSSE tests, Xuelei is working on them - ec tests on solaris-i586,
>>>>>>> they fail even if run standalone
>>>>>>>
>>>>>>> * PKCS11/EC tests:
>>>>>>>
>>>>>>> - New Providers.setAt, to ensure provider is added to desired
>>>>>>> position: test/sun/security/pkcs11/ec/*
>>>>>>>
>>>>>>> - PKCS11Test.safeReload, to ensure shared library can be loaded
>>>>>>> again: test/sun/security/pkcs11/PKCS11Test.java
>>>>>>> test/sun/security/pkcs11/SecmodTest.java
>>>>>>>
>>>>>>> - Test using config files need to run in othervm:
>>>>>>> test/sun/security/pkcs11/Secmod/* test/sun/security/pkcs11/fips/*
>>>>>>>
>>>>>>> - Restoring provider lists: test/sun/security/ec/TestEC.java
>>>>>>> test/sun/security/pkcs11/PKCS11Test.java
>>>>>>>
>>>>>>> * Try-with-resources updates:
>>>>>>>
>>>>>>> - test/com/sun/security/auth/login/ConfigFile/IllegalURL.java -
>>>>>>> test/sun/security/pkcs12/PKCS12SameKeyId.java -
>>>>>>> test/sun/security/tools/keytool/StartDateTest.java
>>>>>>>
>>>>>>> * Other tests still need to be in -othervm:
>>>>>>>
>>>>>>> Using special policy file:
>>>>>>> test/sun/security/provider/PolicyFile/Comparator.java AlgorithmId
>>>>>>> static field oidTable initialization:
>>>>>>> test/sun/security/x509/AlgorithmId/ExtensibleAlgorithmId.java
>>>>>>>
>>>>>>> * JAAS configuration restore:
>>>>>>>
>>>>>>> -
>>>>>>> test/javax/security/auth/login/LoginContext/ResetConfigModule.java
>>>>>>>
>>>>>>> * Use "localhost" as hostname, to make name resolution fast
>>>>>>>
>>>>>>> - test/sun/security/jgss/spnego/NoSpnegoAsDefMech.java
>>>>>>>
>>>>>>>
>>>>>>> After these changes, JPRT test run (with RunSamevmBatch and no
>>>>>>> JSSE tests) shows most tests are OK. There are still intermittent
>>>>>>> failures which are PKCS11/EC provider related.
>>>>>>>
>>>>>>> Thanks Max
>>>>>>
>>>>
>>>
>