Code review request, CR 7153184: NullPointerException when calling SSLEngineImpl.getSupportedCipherSuites
Xuelei Fan
xuelei.fan at oracle.com
Sat May 5 00:33:44 UTC 2012
Thanks for the review.
The changeset is integrated as:
http://hg.openjdk.java.net/jdk8/tl/jdk/rev/41d3f7509e00
Thanks,
Xuelei
On 5/4/2012 7:40 PM, Weijun Wang wrote:
> OK, go on with your code changes, I just think that writing "// The
> maintenance of ciphet suites needs to be synchronized." before a
> synchronized keyword is a little redundant.
>
> BTW, the line above has a typo, s/ciphet/cipher/.
>
> -Max
>
>
> On 05/04/2012 04:51 PM, Xuelei Fan wrote:
>> 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