Moreover, AFAICT, in SharedSecrets, those fields were all immutable -- no state. I spot checked a couple and saw no fields at all. Even final fields would be fine because of "effectively final" classes (e.g. String). But an int array is another matter; no?
~ David Smiley Apache Lucene/Solr Search Developer http://www.linkedin.com/in/davidwsmiley On Tue, Mar 16, 2021 at 6:30 PM Uwe Schindler <[email protected]> wrote: > Hi, > > it is theoretically possible (but unlikely). There was a discussion on the > mailinglist of openjdk, because SharedSecrets had the same problem. > > There's a workaround for that: Use a local variable (copy value to a local > variable, then test for null and run the initialization)! See the fix for > SharedSecrets in the JDK: > https://github.com/openjdk/jdk/commit/85bac8c4 > The trick is here: The order must be ensured in the actual thread, so the > local variable is exactly what you expect and cannot suddenly change to > null by reordering. So when you copy the global to a local and return that > one at end, you know that it's as you expect. > > You could also add a fence, so the order is preserved, I think for that > the following is fine (put it before the write inside the if clause): > < > https://docs.oracle.com/javase/9/docs/api/java/lang/invoke/VarHandle.html#acquireFence-- > > > > I'd go with what the JDK developers did. > > Uwe > > ----- > Uwe Schindler > Achterdiek 19, D-28357 Bremen > https://www.thetaphi.de > eMail: [email protected] > > > -----Original Message----- > > From: David Smiley <[email protected]> > > Sent: Tuesday, March 16, 2021 8:56 PM > > To: [email protected] > > Cc: Michael Gibney <[email protected]> > > Subject: Thread-safety without volatile for an immutable array > > > > I'm code reviewing something over here: > > > > https://github.com/apache/solr/pull/2/files#diff- > > aa2f75bf39a49e1fcd52aacff89da3ea93f622803eb5996d5edb5e04070a50b1R6 > > 58 > > > > 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 > >
