Sent from my iPad
On Nov 27, 2011, at 9:49 AM, Weijun Wang <weijun.w...@oracle.com> wrote: > >> Is it possible that the exception is thrown sometimes but not always? > > If it always fails, there is no problem failing once at the beginning. > Otherwise, if there is a chance it goes fine again after calling some codes, > maybe it's better to try call it each time? > I don't think it the case will happen, otherwise there must be a potential bug. Indeed, the exception is unlikely to happen once JSSE boot up. Xuelei > Max > > On 11/27/2011 09:05 AM, Xuelei Fan wrote: >> >> >> Sent from my iPad >> >> On Nov 27, 2011, at 12:23 AM, Weijun Wang<weijun.w...@oracle.com> wrote: >> >>> There is one thing I'm not sure about: >>> >>> In which case(s), the GeneralSecurityException will be thrown? >> SSLContext initializatio >> >>> Is it possible that the exception is thrown sometimes but not always? Also, >>> the old code has the exception as a clause of the CertStoreException. Can >>> you also include it? >>> >> It is reorged in line 100 and 101, the cause is described as unable to >> initialize the ssl context. >> >> Xuelei >> >>> Max >>> >>> >>> >>> On Nov 26, 2011, at 11:25 PM, Xuelei Fan<xuelei....@oracle.com> wrote: >>> >>>> 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 >>>>>>>> >>>>>>> >>>>>> >>>>
