Unless this code is used millions of millions of times I would just use a
code pattern that is understood by any developer - senior and junior... A
volatile or even initialization under a plain monitor. Those fancy memory
guards are great for low-level data structures but they are really hard to
understand (even for folks who have been using Java for long).

D.

On Wed, Mar 17, 2021 at 4:22 AM David Smiley <[email protected]> wrote:

> 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
> >
> >
>

Reply via email to