new webrev: http://cr.openjdk.java.net/~xuelei/7115524/webrev.01/
You're right on the synchronization issue. In the new webrev, I also remove the "synchronized" keyword on method. Instead, a synchronization on the static attribute is used. Thanks, Xuelei On 11/26/2011 11:16 PM, Weijun Wang wrote: > > On Nov 26, 2011, at 10:58 PM, Xuelei Fan <xuelei....@oracle.com> wrote: > >> 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. > > But this method is only synchronized on the instance, not the class. You can > create two SSLServerCertStore objects can call this method at the same time. > > Max > >>> >>> 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 >>>> >>> >>
