Code review request, CR 7153184: NullPointerException when calling SSLEngineImpl.getSupportedCipherSuites

Xuelei Fan xuelei.fan at oracle.com
Fri May 4 08:51:13 UTC 2012


On 5/4/2012 4:39 PM, Weijun Wang wrote:
> 
> 
> On 05/04/2012 04:24 PM, Xuelei Fan wrote:
>> On May 4, 2012, at 3:31 PM, Weijun Wang<weijun.wang at oracle.com>  wrote:
>>
>>> The fix is good, but I think you are over-commenting. Everyone seeing
>>> the synchronized keyword knows what it means. You can keep the new
>>> lines at 380-381.
>>>
>> Thanks for the review.  The purpose of the over-commenting is to avoid
>> to use synchronized methods instead of synchronized block in the future.
> 
> You mean you are afraid that someone will add the synchronized keyword
> to the clearAvailableCache method again? You can keep 380-381.
> 
I'm afraid some one move the synchronized keyword to
getSuportedCipherSuiteList method or getDefaultCipherSuiteList method as:

synchronized CipherSuiteList getSuportedCipherSuiteList();
synchronized CipherSuiteList getDefaultCipherSuiteList(boolean);

The above two methods cannot be called at the same time in different
threads.

Thanks,
Xuelei

> -Max
> 
>>
>> Thanks,
>> Xuelei
>>
>>> Thanks
>>> Max
>>>
>>> On 05/04/2012 12:37 PM, Xuelei Fan wrote:
>>>> Hi,
>>>>
>>>> Please review the synchronization issue in SSLContextImpl.
>>>>
>>>> bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7153184
>>>> webrev: http://cr.openjdk.java.net/~xuelei/7153184/webrev.00/
>>>>
>>>> No new regression test, simple fix and hard to reproduce the issue.
>>>>
>>>> Thanks,
>>>> Xuelei




More information about the security-dev mailing list