[concurrency-interest] Double Checked Locking in OpenJDK

Boehm, Hans hans.boehm at hp.com
Tue Aug 14 21:31:41 EDT 2012


Good point.  It may be unavoidable in some cases.  (And I think we need an annotation to have data race checkers ignore such things.)  But it still strikes me as quite brittle, and not something most programmers should be playing with:

1. We know the current JMM semantics for data races are not correct.  We don't really know what the correct semantics are.  Probably this idiom won't break with standard compilers.  But we've been surprised before.

2. Its correctness relies on very subtle properties of the interface to the lazily initialized object.  If we have a get() method that lazily initializes and returns a field, I think it's crucial that the caller of get() has no way to tell whether it was the initializing thread, or whether another thread was.  Otherwise the absence of sequential consistency becomes apparent to the caller.  Adding an is_initialized() method breaks things in similar ways. 

If this were undetectable on 99.99% of systems, I would certainly argue against it.  But I'm not sure that's true on current ARM systems.  (It is on x86.) Nonetheless, I think it only makes sense only if there are good reasons to believe it may be performance critical.

Hans

> -----Original Message-----
> From: Zhong Yu [mailto:zhong.j.yu at gmail.com]
> Sent: Tuesday, August 14, 2012 5:46 PM
> To: Boehm, Hans
> Cc: Vitaly Davidovich; concurrency-interest at cs.oswego.edu
> Subject: Re: [concurrency-interest] Double Checked Locking in OpenJDK
> 
> Lazy initialization is a common use case. Using thread-safe immutable
> objects to avoid a `volatile` read is simple and well understood,
> though it contains data race. I would say it is a common, useful and
> justifiable data race.
> 
> You can argue that a volatile read isn't that expensive, lazy
> initialization with volatile variable won't have any detectable
> performance degrading on 99.99% systems. That may be true. But we
> don't justify each choice of a concurrency tool by "profiling/testing"
> real world scenario - seriously, who can afford that?
> 
> Zhong Yu
> 
> On Tue, Aug 14, 2012 at 7:01 PM, Boehm, Hans <hans.boehm at hp.com> wrote:
> > Indeed.  My view is that we need special final field semantics
> primarily to
> > deal with untrusted code, where you don't have the control to
> guarantee
> > there are no data races.  There has to be a way for untrusted code to
> pass a
> > string to a library in a way that guarantees the string won't change
> between
> > the time a security manager looks at it and the time it's used as a
> file
> > name.  Thus we have to be able to construct classes that are fully
> > immutable, no matter how badly the client mistreats them.  Final
> fields give
> > you a way to do that without adding tons of overhead to a class like
> String.
> >
> >
> >
> > If you can avoid data races, do.
> >
> >
> >
> > Hans
> >
> >
> >
> > From: concurrency-interest-bounces at cs.oswego.edu
> > [mailto:concurrency-interest-bounces at cs.oswego.edu] On Behalf Of
> Vitaly
> > Davidovich
> > Sent: Tuesday, August 14, 2012 3:36 PM
> > To: Zhong Yu
> > Cc: concurrency-interest at cs.oswego.edu
> >
> >
> > Subject: Re: [concurrency-interest] Double Checked Locking in OpenJDK
> >
> >
> >
> > I think the lesson should be avoid data races - period. :) if you
> find a
> > case where you can justify it (i.e. profiling/testing guided) then
> consider
> > it.  I bet there will be few and far in between cases like that.
> >
> > Sent from my phone
> >
> > On Aug 14, 2012 5:02 PM, "Zhong Yu" <zhong.j.yu at gmail.com> wrote:
> >
> > Lesson I learned from this thread:
> >
> > do *not* write thread-safe immutable class, unless there's a good
> reason.
> >
> > One good reason is double checked locking without volatile.
> >
> > Zhong Yu
> >
> > On Tue, Aug 14, 2012 at 3:09 PM, Vitaly Davidovich
> <vitalyd at gmail.com>
> > wrote:
> >> You should use final for its semantic meaning/code clarity unless
> you
> >> yourself are publishing it unsafely in your library.  If users of
> your lib
> >> are sharing this class, they need to provide the required
> >> synchronization/hand off.  That's my personal view (and I always try
> to
> >> use
> >> final as much as possible, but mostly for clarity/code semantics).
> >>
> >> For people running the jvm on an arch where final is not just
> compiler
> >> barrier, I agree it's a bit of a problem - really, final should not
> carry
> >> both java semantic and JMM meaning, but I can see why that was done.
> >>
> >> Sent from my phone
> >>
> >> On Aug 14, 2012 3:58 PM, "Zhong Yu" <zhong.j.yu at gmail.com> wrote:
> >>>
> >>> On Tue, Aug 14, 2012 at 2:52 PM, Vitaly Davidovich
> <vitalyd at gmail.com>
> >>> wrote:
> >>> > Yes, presumably on some archs with very weak memory order it
> could
> >>> > cause
> >>> > some performance impact.  On TSO, it's just a compiler barrier
> >>> > preventing
> >>> > code motion that would publish the reference before constructor
> >>> > completes,
> >>> > but on those weak archs it could also mean a hardware fence.
> Whether
> >>> > the
> >>> > tradeoff is worth it in general or not is debatable.  You can't
> have it
> >>> > both
> >>> > ways, I agree, but the jvm and JMM give you options and guidance
> on how
> >>> > to
> >>> > do the "right" thing.
> >>>
> >>> If I'm writing a Java library, containing a Point(x,y) class, which
> is
> >>> immutable (in the narrower sense), should I use final fields or
> not?
> >>> What's the "right" thing to do?
> >>>
> >>> If the answer is platform dependent, oh well.
> >>>
> >>> Zhong Yu
> >>>
> >>> >
> >>> > Sent from my phone
> >>> >
> >>> > On Aug 14, 2012 3:47 PM, "Zhong Yu" <zhong.j.yu at gmail.com> wrote:
> >>> >>
> >>> >> Are `final` fields a problem to compiler optimization?
> >>> >>
> >>> >> You can't have both ways.
> >>> >>
> >>> >> Either `final` field semantics is good and cheap, therefore it
> should
> >>> >> be extended to all fields.
> >>> >>
> >>> >> Or there is some downsides with 'final' field semantics, so we
> >>> >> shouldn't encourage people to apply `final` whenever they can.
> Lets
> >>> >> hear those downsides.
> >>> >>
> >>> >> Zhong Yu
> >>> >>
> >>> >> On Tue, Aug 14, 2012 at 2:42 PM, Vitaly Davidovich
> <vitalyd at gmail.com>
> >>> >> wrote:
> >>> >> > This is only an issue when publishing unsafely.  Allowing
> treating
> >>> >> > constructors as regular methods is a good thing as it gives
> the
> >>> >> > compiler
> >>> >> > a
> >>> >> > chance to optimize code, which everyone likes and benefits
> from.
> >>> >> >
> >>> >> > Sent from my phone
> >>> >> >
> >>> >> > On Aug 14, 2012 3:39 PM, "Zhong Yu" <zhong.j.yu at gmail.com>
> wrote:
> >>> >> >>
> >>> >> >> From a user's point of view, shouldn't constructors be
> special
> >>> >> >> though?
> >>> >> >> An object shouldn't be considered in existence until its
> >>> >> >> construction
> >>> >> >> is done; it is pathological that some outsider can observe a
> >>> >> >> partially
> >>> >> >> constructed object. Life is simpler if we can eliminate that
> >>> >> >> possibility (unless `this` is leaked inside constructor)
> >>> >> >>
> >>> >> >> Zhong Yu
> >>> >> >>
> >>> >> >> On Tue, Aug 14, 2012 at 1:58 PM, Nathan Reynolds
> >>> >> >> <nathan.reynolds at oracle.com> wrote:
> >>> >> >> > We seem to be splitting two notions (i.e thread-safe and
> safe
> >>> >> >> > publication)
> >>> >> >> > when they should be combined in a sense.  Typically, when
> we say
> >>> >> >> > thread-safe
> >>> >> >> > we talk about the operations performed on the object after
> it was
> >>> >> >> > constructed (and its contents are globally visible).
> However, we
> >>> >> >> > need
> >>> >> >> > to
> >>> >> >> > consider that executing the constructor is modifying the
> state of
> >>> >> >> > the
> >>> >> >> > object.  It requires the same mechanisms that the rest of
> the
> >>> >> >> > class
> >>> >> >> > uses
> >>> >> >> > to
> >>> >> >> > ensure thread-safety.  Even though, there is only 1 thread
> >>> >> >> > executing
> >>> >> >> > the
> >>> >> >> > constructor, a proper releasing of a lock or some other
> >>> >> >> > happens-before
> >>> >> >> > construct is required to ensure that the memory updates by
> the
> >>> >> >> > thread
> >>> >> >> > are
> >>> >> >> > made globally visible before the object is accessed by
> another
> >>> >> >> > thread.
> >>> >> >> > This
> >>> >> >> > is what we are calling safe publication.  So, safe
> publication is
> >>> >> >> > a
> >>> >> >> > subset
> >>> >> >> > of thread-safety except it is limited to what happens after
> the
> >>> >> >> > constructor
> >>> >> >> > is called and before the object is used by multiple
> threads.
> >>> >> >> >
> >>> >> >> > A beautifully-written class can be thread-safe with respect
> to
> >>> >> >> > calling
> >>> >> >> > its
> >>> >> >> > member methods but not thread-safe with respect to calling
> its
> >>> >> >> > constructor.
> >>> >> >> > It is this latter case that many stumble upon because they
> think
> >>> >> >> > that
> >>> >> >> > constructors are inherently thread-safe because they are
> executed
> >>> >> >> > single-threadedly.  What they fail to realize is that the
> >>> >> >> > execution
> >>> >> >> > of a
> >>> >> >> > constructor can overlap with the execution of other code
> from the
> >>> >> >> > view
> >>> >> >> > point
> >>> >> >> > of what is happening in memory.  This same problem applies
> to
> >>> >> >> > more
> >>> >> >> > rare
> >>> >> >> > case
> >>> >> >> > of regular methods which can be proven to execute in a
> single
> >>> >> >> > thread
> >>> >> >> > but
> >>> >> >> > don't use synchronization before multiple threads start
> accessing
> >>> >> >> > the
> >>> >> >> > shared
> >>> >> >> > data.
> >>> >> >> >
> >>> >> >> > Nathan Reynolds | Consulting Member of Technical Staff |
> >>> >> >> > 602.333.9091
> >>> >> >> > Oracle PSR Engineering | Server Technology
> >>> >> >> > On 8/13/2012 4:08 PM, David Holmes wrote:
> >>> >> >> >
> >>> >> >> > Ruslan Cheremin writes:
> >>> >> >> >
> >>> >> >> > For me it is confusing: java has only one way to have
> really
> >>> >> >> > immutable
> >>> >> >> > object, and this way also gives you a total thread safety
> even
> >>> >> >> > for
> >>> >> >> > data race based publication. But then docs refer object as
> >>> >> >> > "immutable
> >>> >> >> > and thread-safe" -- we still can't assume it to be really
> >>> >> >> > thread-safe?
> >>> >> >> >
> >>> >> >> > It is better/simpler to isolate the notion of thread-safety
> and
> >>> >> >> > safe
> >>> >> >> > publication. Thread-safety comes into play after you have
> safely
> >>> >> >> > shared
> >>> >> >> > an
> >>> >> >> > object. The means by which you safely share an object is
> >>> >> >> > orthogonal
> >>> >> >> > to
> >>> >> >> > how
> >>> >> >> > the object itself is made thread-safe.
> >>> >> >> >
> >>> >> >> > The means by which an object is shared has to involve
> shared
> >>> >> >> > mutable
> >>> >> >> > state,
> >>> >> >> > and use of shared mutable state always needs some form of
> >>> >> >> > synchronization
> >>> >> >> > (either implicit eg due to static initialization; or
> explicit by
> >>> >> >> > using
> >>> >> >> > volatile or synchronized getter/setter methods).
> >>> >> >> >
> >>> >> >> > David
> >>> >> >> > -----
> >>> >> >> >
> >>> >> >> > It's a pity, especially because true immutability gives us
> some
> >>> >> >> > chances of performance optimization. As in this case -- we
> do not
> >>> >> >> > really need .path to be volatile here, if we would assume
> Path to
> >>> >> >> > be
> >>> >> >> > truly immutable. volatility here required only for ensuring
> safe
> >>> >> >> > publishing.
> >>> >> >> >
> >>> >> >> > 2012/8/13 David Holmes <davidcholmes at aapt.net.au>:
> >>> >> >> >
> >>> >> >> > Ruslan Cheremin writes:>
> >>> >> >> >
> >>> >> >> > But is there a way to define "safe for data race
> publishing"? I
> >>> >> >> > as
> >>> >> >> > far, as I remember, "immutable and thread-safe" is standard
> >>> >> >> > mantra
> >>> >> >> > in
> >>> >> >> > JDK javadocs for totally safe objects. j.l.String has same
> mantra
> >>> >> >> > --
> >>> >> >> > and it is safe for any way of publishing. Does you mean, I
> should
> >>> >> >> > explicitly add "safe even for publishing via data race" in
> docs?
> >>> >> >> > But
> >>> >> >> > I
> >>> >> >> > can't remember any such phrase in JDK docs.
> >>> >> >> >
> >>> >> >> > I don't recall anything in the JDK docs that mention being
> >>> >> >> >
> >>> >> >> > "totally safe"
> >>> >> >> >
> >>> >> >> > regardless of publication mechanism. Some classes, eg
> String,
> >>> >> >> > have
> >>> >> >> > been
> >>> >> >> > defined such that they do have that property (for security
> >>> >> >> > reasons).
> >>> >> >> > In
> >>> >> >> > general neither "thread-safe" nor "immutable" imply
> >>> >> >> > safe-for-unsynchronized-publication.
> >>> >> >> >
> >>> >> >> > Java Concurrency In Practice (jcip.net) does define
> additional
> >>> >> >> > potential
> >>> >> >> > annotations, where @Immutable would indeed capture the
> >>> >> >> > requirement
> >>> >> >> > of
> >>> >> >> > safe-for-unsynchronized-publication.
> >>> >> >> >
> >>> >> >> > David
> >>> >> >> > -----
> >>> >> >> >
> >>> >> >> > 2012/8/13 David Holmes <davidcholmes at aapt.net.au>:
> >>> >> >> >
> >>> >> >> > Ruslan Cheremin writes:
> >>> >> >> >
> >>> >> >> > Well, Path javadoc explicitly says "immutable and safe for
> >>> >> >> > multithreaded use". Although it is not strictly defined in
> java
> >>> >> >> > what
> >>> >> >> > exactly means "safe for multithreaded use" -- does it mean
> safe
> >>> >> >> > for
> >>> >> >> > publishing via data race, among others? -- I suppose, it
> >>> >> >> >
> >>> >> >> > should be. Am
> >>> >> >> >
> >>> >> >> > I wrong here?
> >>> >> >> >
> >>> >> >> > "safe for multi-threaded use" does not generally imply that
> it
> >>> >> >> >
> >>> >> >> > is safe to
> >>> >> >> >
> >>> >> >> > publish instances without synchronization of some form.
> >>> >> >> >
> >>> >> >> > David
> >>> >> >> > -----
> >>> >> >> >
> >>> >> >> > From other side, File.toPath javadoc explicitly says what
> >>> >> >> > "returned
> >>> >> >> > instance must be the same for every invocation", so sync
> block is
> >>> >> >> > required here for mutual exclusion on initialization phase.
> >>> >> >> > Without
> >>> >> >> > this requirement it is also safe to live without sync
> block,
> >>> >> >> > afaik.
> >>> >> >> >
> >>> >> >> > 2012/8/13 David Holmes <davidcholmes at aapt.net.au>:
> >>> >> >> >
> >>> >> >> > Ruslan Cheremin writes:
> >>> >> >> >
> >>> >> >> > First of all, Path is immutable, so DCL is safe here even
> without
> >>> >> >> > volatile. Volatile here is not required from my point of
> view.
> >>> >> >> >
> >>> >> >> > Without the volatile the Path implementation (Path is an
> >>> >> >> >
> >>> >> >> > interface) must be
> >>> >> >> >
> >>> >> >> > such that an instance of Path can be safely published
> without
> >>> >> >> >
> >>> >> >> > any additional
> >>> >> >> >
> >>> >> >> > forms of synchronization. Immutability does not in itself
> >>> >> >> >
> >>> >> >> > ensure that. You
> >>> >> >> >
> >>> >> >> > would have to examine the actual implementation class.
> >>> >> >> >
> >>> >> >> > David Holmes
> >>> >> >> > ------------
> >>> >> >> >
> >>> >> >> >
> >>> >> >> > 2012/8/12 Dmitry Vyazelenko <vyazelenko at yahoo.com>:
> >>> >> >> >
> >>> >> >> > Hi Richard,
> >>> >> >> >
> >>> >> >> > The variable "filePath" is volatile, so the double-checked
> >>> >> >> >
> >>> >> >> > locking is correct in this case. It would have been a bug
> >>> >> >> >
> >>> >> >> > prior to Java 5.
> >>> >> >> >
> >>> >> >> > Best regards,
> >>> >> >> >
> >>> >> >> > Dmitry Vyazelenko
> >>> >> >> >
> >>> >> >> > On Aug 12, 2012, at 21:35 , Richard Warburton
> >>> >> >> >
> >>> >> >> > <richard.warburton at gmail.com> wrote:
> >>> >> >> >
> >>> >> >> > Hello,
> >>> >> >> >
> >>> >> >> > The current implementation of java.io.File::toPath [0]
> >>> >> >> >
> >>> >> >> > appears to be
> >>> >> >> >
> >>> >> >> > using the double checked locking pattern:
> >>> >> >> >
> >>> >> >> >     public Path toPath() {
> >>> >> >> >         Path result = filePath;
> >>> >> >> >         if (result == null) {
> >>> >> >> >             synchronized (this) {
> >>> >> >> >                 result = filePath;
> >>> >> >> >                 if (result == null) {
> >>> >> >> >                     result =
> >>> >> >> >
> >>> >> >> > FileSystems.getDefault().getPath(path);
> >>> >> >> >
> >>> >> >> >                     filePath = result;
> >>> >> >> >                 }
> >>> >> >> >             }
> >>> >> >> >         }
> >>> >> >> >         return result;
> >>> >> >> >     }
> >>> >> >> >
> >>> >> >> > I was going to report the bug, but I'm a little
> >>> >> >> >
> >>> >> >> > uncertain of the
> >>> >> >> >
> >>> >> >> > interaction between the local variable 'result' and DCL
> >>> >> >> >
> >>> >> >> > since I've
> >>> >> >> >
> >>> >> >> > previously only seen the checking condition on the
> >>> >> >> >
> >>> >> >> > shared field
> >>> >> >> >
> >>> >> >> > itself.  Can someone here either confirm that its a bug or
> >>> >> >> >
> >>> >> >> > explain how
> >>> >> >> >
> >>> >> >> > the 'result' variable is fixing things?
> >>> >> >> >
> >>> >> >> > regards,
> >>> >> >> >
> >>> >> >> >  Richard
> >>> >> >> >
> >>> >> >> > [0] See the end of
> >>> >> >> >
> >>> >> >> >
> >>> >> >> >
> hg.openjdk.java.net/jdk8/jdk8/jdk/file/da8649489aff/src/share/clas
> >>> >> >> >
> >>> >> >> > ses/java/io/File.java
> >>> >> >> >
> >>> >> >> > _______________________________________________
> >>> >> >> > Concurrency-interest mailing list
> >>> >> >> > Concurrency-interest at cs.oswego.edu
> >>> >> >> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
> >>> >> >> >
> >>> >> >> > _______________________________________________
> >>> >> >> > Concurrency-interest mailing list
> >>> >> >> > Concurrency-interest at cs.oswego.edu
> >>> >> >> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
> >>> >> >> >
> >>> >> >> > _______________________________________________
> >>> >> >> > Concurrency-interest mailing list
> >>> >> >> > Concurrency-interest at cs.oswego.edu
> >>> >> >> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
> >>> >> >> >
> >>> >> >> > _______________________________________________
> >>> >> >> > Concurrency-interest mailing list
> >>> >> >> > Concurrency-interest at cs.oswego.edu
> >>> >> >> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
> >>> >> >> >
> >>> >> >> >
> >>> >> >> >
> >>> >> >> > _______________________________________________
> >>> >> >> > Concurrency-interest mailing list
> >>> >> >> > Concurrency-interest at cs.oswego.edu
> >>> >> >> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
> >>> >> >> >
> >>> >> >> _______________________________________________
> >>> >> >> Concurrency-interest mailing list
> >>> >> >> Concurrency-interest at cs.oswego.edu
> >>> >> >> http://cs.oswego.edu/mailman/listinfo/concurrency-interest



More information about the Concurrency-interest mailing list