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

[Weijun Wang]

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
>