2nd round code review request: 7055363: jdk_security3 cleanup
Weijun Wang
weijun.wang at oracle.com
Thu Aug 4 09:58:42 UTC 2011
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? 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
>>>>
>>
>
More information about the security-dev
mailing list