On 11/27/2011 05:48 PM, Xuelei Fan wrote:
Sent from my iPad
On Nov 27, 2011, at 5:01 PM, Weijun Wang<weijun.w...@oracle.com> wrote:
Anyway, I find this a little unsafe. One can add a new SSL security provider at
run time, and then run this method, and the new provider can be loaded.
I did not follow your ideas. The new provider is loaded, and what's the worries
then?
Typo, I meant "cannot".
1. Run this method, the default SSL provider loaded
2. Add a new SSL security provider
3. Run this method again, still the old provider.
Max
Xuelei
In my opinion, this is more important than performance.
Thanks
Max
On 11/27/2011 10:15 AM, Xuelei Fan wrote:
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
