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<[email protected]> 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
