Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.java.programmer > #30474

Re: SingletonFactory and safe publication

Show all headers | View raw

On 12/2/2014 6:52 PM, Arne Vajhøj wrote:
> On 12/2/2014 6:20 PM, Eric Sosman wrote:
>> On 12/1/2014 9:16 PM, John wrote:
>>> Hi:
>>>
>>> I am reading this
>>> article(http://shipilev.net/blog/2014/safe-public-construction/). It
>>> says the following code is GOOD:
>>>
>>> public class SafeDCLFactory {
>>>    private volatile Singleton instance;
>>>
>>>    public Singleton get() {
>>>      if (instance == null) {  // check 1
>>>        synchronized(this) {
>>>          if (instance == null) { // check 2
>>>            instance = new Singleton();
>>>          }
>>>        }
>>>      }
>>>      return instance;
>>>    }
>>> }
>>>
>>> I feel disagree, by learning from this
>>> article(http://en.wikipedia.org/wiki/Double-checked_locking).
>>
>>      Brian Goetz agrees that this code is incorrect, calling it
>> "a commonly suggested nonfix."  He explains that although the
>> accesses to `instance' will be consistent because `volatile'
>> ensures it, any accesses to the member variables of the new
>> Singleton are *not* consistent (unless they are `volatile', too).
>> You could get a sequence like this:
>>
>>      Thread T1 finds `instance' null, obtains the lock, finds
>>      that `instance' is still null, and calls the constructor.
>>
>>      The constructor (running in T1) stores initial values in
>>      the member variables of the new Singleton.  We presume
>>      that at least some of these variables are not `volatile'.
>>
>>      The constructor finishes, and now T1 stores the new
>>      reference to `instance'.  Because `instance' is `volatile',
>>      T1 ensures that the new value is actually flushed from
>>      store buffers and write caches and so on, and appears in
>>      stable memory.
>>
>>      Thread T2 now finds `instance' non-null, and starts using
>>      it to refer to the Singleton's member variables (either
>>      directly or by calling the Singleton's methods).
>>
>>      Unfortunately, the values stored by Singleton's constructor
>>      may still be sitting in caches and what-not, and may not yet
>>      have been flushed to stable memory.  Even if the constructor
>>      running in T1 stored 42 in some member variable, T2 may
>>      read the value as zero.
>>
>>      ... because there is no "happens-before" between T1's storing
>>      of the value and T2's reading of it.
>>
>>      In short, making sure that `instance' is safe is not sufficient;
>> you also need to worry about everything `instance' refers to, directly
>> or indirectly.
>>
>> http://www.javaworld.com/article/2074979/java-concurrency/double-checked-locking--clever--but-broken.html
>>
>
> But that is with the pre-5 memory model (the article is from 2001).
>
> They changed the treatment of volatile in the memory model
> in Java 5.
>
> Or have I misunderstood something?

     After a fair amount of studying JLS Chapter 17, my conclusion is:
"My head hurts."  :(

     17.4 describes the memory model, and the first part is not too
difficult for my comprehension: actions, program order, synchronization
order, happens-before order -- all these make sense to me.  Somewhere
near the end of 17.4.5, though, I start to bog down: The JLS starts
talking about whether a read is "allowed" to see the effect of a write,
whether a set of actions is "happens-before consistent," whether an
execution is "well-formed," and so on.  Just when it looked (to me)
like we'd reached the end of the story by defining happens-before,
the story continues in a way that makes my head ache ...

     So I turned to the less formal but more readable "Java Concurrency
in Practice" by Goetz et al., and found something understandable:

	"Subsequent changes in the JMM (Java 5.0 and later) have enabled
	DCL to work if `resource' is made `volatile', ..."

... indicating that you're right and I was wrong.  Thanks for the
correction.

     However, Goetz & Co. also opine that DCL is less useful now than
it once was (ironic: It was useful when it didn't work, and now that
it works it's useless).  Uncontended synchronization, they say, is a
lot faster than it used to be, so working hard to avoid it is effort
misdirected.  Instead, they recommend the shorter/simpler/safer
"holder class" method, here paraphrased in the O.P.'s terms:

	public class SingletonFactory {
	    private static class SingletonHolder {
	        static final Singleton instance = new Singleton();
	    }
	    public Singleton get() {
	        return SingletonHolder.instance;
	    }
	}

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

Back to comp.lang.java.programmer | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

SingletonFactory and safe publication John <xsli2@yahoo.com> - 2014-12-01 18:16 -0800
  Re: SingletonFactory and safe publication Arne Vajhøj <arne@vajhoej.dk> - 2014-12-01 21:34 -0500
  Re: SingletonFactory and safe publication Eric Sosman <esosman@comcast-dot-net.invalid> - 2014-12-02 18:20 -0500
    Re: SingletonFactory and safe publication Arne Vajhøj <arne@vajhoej.dk> - 2014-12-02 18:52 -0500
      Re: SingletonFactory and safe publication Eric Sosman <esosman@comcast-dot-net.invalid> - 2014-12-03 09:51 -0500
        Re: SingletonFactory and safe publication Arne Vajhøj <arne@vajhoej.dk> - 2014-12-03 21:29 -0500
        Re: SingletonFactory and safe publication Arne Vajhøj <arne@vajhoej.dk> - 2014-12-03 21:34 -0500
          Re: SingletonFactory and safe publication Robert Klemme <shortcutter@googlemail.com> - 2014-12-08 08:48 +0100
            Re: SingletonFactory and safe publication Lew <lewbloch@gmail.com> - 2015-01-04 16:15 -0800

csiph-web