Hi,
I agree with Dawid to make it developer friendly. Most synchronized-blocks are
rmeoved by Hotspot anyways, so better safe than sorry!
David: I missed in your original mail that you were afraid of changes inside
the array not being visible. You can prevent that with fences, but all of that
is hard to understand.
IMHO, I would use an AtomicReference:
final AtomicReference<V> arr = new AtomicReference<>();
public V getLazy() {
V result = arr.get();
if (result == null) {
result = compute(...);
if (!arr.compareAndSet(null, result)) {
return arr.get();
}
}
return result;
}
-----
Uwe Schindler
Achterdiek 19, D-28357 Bremen
https://www.thetaphi.de
eMail: [email protected]
> -----Original Message-----
> From: Dawid Weiss <[email protected]>
> Sent: Wednesday, March 17, 2021 9:01 AM
> To: [email protected]
> Subject: Re: Thread-safety without volatile for an immutable array
>
> 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#a
> cquireFence--
> > > >
> > >
> > > 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
> > >
> > >
> >
