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 >> >