[concurrency-interest] Double Checked Locking in OpenJDK

Boehm, Hans hans.boehm at hp.com
Tue Aug 14 20:01:18 EDT 2012


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<mailto: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<mailto: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<mailto:zhong.j.yu at gmail.com>> wrote:
>>
>> On Tue, Aug 14, 2012 at 2:52 PM, Vitaly Davidovich <vitalyd at gmail.com<mailto: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<mailto: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<mailto: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<mailto: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<mailto: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<tel: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<mailto: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<http://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<mailto: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<mailto: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<mailto: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<mailto: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<http://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<mailto:Concurrency-interest at cs.oswego.edu>
>> >> >> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>> >> >> >
>> >> >> > _______________________________________________
>> >> >> > Concurrency-interest mailing list
>> >> >> > Concurrency-interest at cs.oswego.edu<mailto:Concurrency-interest at cs.oswego.edu>
>> >> >> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>> >> >> >
>> >> >> > _______________________________________________
>> >> >> > Concurrency-interest mailing list
>> >> >> > Concurrency-interest at cs.oswego.edu<mailto:Concurrency-interest at cs.oswego.edu>
>> >> >> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>> >> >> >
>> >> >> > _______________________________________________
>> >> >> > Concurrency-interest mailing list
>> >> >> > Concurrency-interest at cs.oswego.edu<mailto:Concurrency-interest at cs.oswego.edu>
>> >> >> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> > _______________________________________________
>> >> >> > Concurrency-interest mailing list
>> >> >> > Concurrency-interest at cs.oswego.edu<mailto:Concurrency-interest at cs.oswego.edu>
>> >> >> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>> >> >> >
>> >> >> _______________________________________________
>> >> >> Concurrency-interest mailing list
>> >> >> Concurrency-interest at cs.oswego.edu<mailto:Concurrency-interest at cs.oswego.edu>
>> >> >> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20120815/c403781d/attachment-0001.html>


More information about the Concurrency-interest mailing list