lucene-solr icon indicating copy to clipboard operation
lucene-solr copied to clipboard

SOLR-13705 Double-checked locking bug is fixed.

Open kamaci opened this issue 6 years ago • 2 comments
trafficstars

Description

Using double-checked locking for the lazy initialization of any other type of primitive or mutable object risks a second thread using an uninitialized or partially initialized member while the first thread is still creating it, and crashing the program.

Solution

Double-checked locking bug is fixed to prevent it.

Tests

No need to write extra tests. Existing tests should not fail after this implementation.

Checklist

Please review the following and check all that apply:

  • [X] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • [X] I have created a Jira issue and added the issue ID to my pull request title.
  • [X] I am authorized to contribute this code to the ASF and have removed any code I do not have a license to distribute.
  • [X] I have developed this patch against the master branch.
  • [X] I have run ant precommit and the appropriate test suite.
  • [ ] I have added tests for my changes.
  • [ ] I have added documentation for the Ref Guide (for Solr changes only).

kamaci avatar Aug 20 '19 12:08 kamaci

Thanks for contributing! Is there a reason you didn't check the authorization checkbox in the template?

Your description mentions a mutable object but I think it's the immutability here that enables us to avoid a volatile. Speaking of which I looked closer to see if these lazily created objects are immutable. SSLConfigurations maybe has an issue. It looks okay but it's only field is an SSLCredentialProviderFactory that should be immutable but technically lacks it's only field, a String providerChain, from being declared final. That is required for "publication safety" I think. Furthermore I think the immutability of these things should be declared with a comment as justification for the lack of volatile.

Reference: https://wiki.sei.cmu.edu/confluence/display/java/LCK10-J.+Use+a+correct+form+of+the+double-checked+locking+idiom Reference: https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

dsmiley avatar Sep 04 '19 04:09 dsmiley

+1 to the code you have here.

dsmiley avatar Sep 04 '19 12:09 dsmiley