I'm code reviewing something over here:

https://github.com/apache/solr/pull/2/files#diff-aa2f75bf39a49e1fcd52aacff89da3ea93f622803eb5996d5edb5e04070a50b1R658

In a nutshell, it looks like this:

  private int[] cachedOrdIdxMap;

  private int[] getOrdIdxMap(...) {
    if (cachedOrdIdxMap == null) {
      cachedOrdIdxMap = compute(...); // idempotent (same result for same
input)
    }
    return cachedOrdIdxMap;
  }

I suspect this is not thread-safe because the value is not "published"
safely, and thus it might be possible for a reader to see a
non-null cachedOrdIdxMap yet it might not be populated yet because the
machine is allowed to re-order reads and writes in the absence of any
synchronization controls.  The fix would be declaring the array volatile, I
think.

Thoughts?

~ David Smiley
Apache Lucene/Solr Search Developer
http://www.linkedin.com/in/davidwsmiley

Reply via email to