Code review request, 7115524: sun.security.provider.certpath.ssl.SSLServerCertStore no longer works

[Xuelei Fan]

I was wondering why we synchronized the getInstance() and
engineGetCertificates(). It seems that we only need to synchronize the
serverChain variable.

I will update the synchronization if no objection.

Thanks,
Xuelei

On 11/26/2011 10:42 PM, Xuelei Fan wrote:
> On 11/26/2011 9:59 PM, Weijun Wang wrote:
>> Hi Xuelei
>>
>> Why move the local variables to static fields?
>>
> In order to improve the performance. SSLContext.init() is expensive.
> 
>> The engineGetCertificates method is only synchronized on this but not the whole class, so there is a chance that the method is called by multiple threads at the same time. This means if you only use one static GetChainTrustManager field, its serverChain field can be modified by different threads.
>>
> We have synchronized the engineGetCertificates(), and this method is the
> only method to access the GetChainTrustManager object. As the attribute
> is declared as "final static", so it will be initialized once even in
> multiple threads.
> 
> That's, the access to serverChain variable has been synchronized by
> synchronizing the engineGetCertificates method.
> 
> Xuelei
> 
>> -Max
>>
>> On Nov 26, 2011, at 8:43 PM, Xuelei Fan wrote:
>>
>>> webrev: http://cr.openjdk.java.net/~xuelei/7115524/webrev.00/
>>>
>>> No new regression test, the current test,
>>> test/sun/security/tools/keytool/printssl.sh, has already been able to
>>> test the update.
>>>
>>> Thanks,
>>> Xuelei
>>
>