Skip to end of metadata
Go to start of metadata

The double-checked locking idiom is a software design pattern used to reduce the overhead of acquiring a lock by first testing the locking criterion without actually acquiring the lock. Double-checked locking improves performance by limiting synchronization to the rare case of computing the field's value or constructing a new instance for the field to reference and by foregoing synchronization during the common case of retrieving an already-created instance or value.

Incorrect forms of the double-checked locking idiom include those that allow publication of an uninitialized or partially initialized object. Consequently, only those forms of the double-checked locking idiom that correctly establish a happens-before relationship both for the helper reference and for the complete construction of the Helper instance are permitted.

The double-checked locking idiom is frequently used to implement a singleton factory pattern that performs lazy initialization. Lazy initialization defers the construction of a member field or an object referred to by a member field until an instance is actually required rather than computing the field value or constructing the referenced object in the class's constructor. Lazy initialization helps to break harmful circularities in class and instance initialization. It also enables other optimizations [Bloch 2005].

Lazy initialization uses either a class or an instance method, depending on whether the member object is static. The method checks whether the instance has already been created and, if not, creates it. When the instance already exists, the method simply returns the instance:

// Correct single threaded version using lazy initialization
final class Foo {
  private Helper helper = null;

  public Helper getHelper() {
    if (helper == null) {
      helper = new Helper();
    }
    return helper;
  }
  // ...
}

Lazy initialization must be synchronized in multithreaded applications to prevent multiple threads from creating extraneous instances of the member object:

// Correct multithreaded version using synchronization
final class Foo {
  private Helper helper = null;

  public synchronized Helper getHelper() {
    if (helper == null) {
      helper = new Helper();
    }
    return helper;
  }
  // ...
}

Noncompliant Code Example

The double-checked locking pattern uses block synchronization rather than method synchronization and installs an additional null reference check before attempting synchronization. This noncompliant code example uses an incorrect form of the double-checked locking idiom:

// Double-checked locking idiom
final class Foo {
  private Helper helper = null;
  public Helper getHelper() {
    if (helper == null) {
      synchronized (this) {
        if (helper == null) {
          helper = new Helper();
        }
      }
    }
    return helper;
  }

  // Other methods and members...
}

According to Pugh [Pugh 2004],

Writes that initialize the Helper object and the write to the helper field can be done or perceived out of order. As a result, a thread which invokes getHelper() could see a non-null reference to a helper object, but see the default values for fields of the helper object, rather than the values set in the constructor.

Even if the compiler does not reorder those writes, on a multiprocessor, the processor or the memory system may reorder those writes, as perceived by a thread running on another processor.

This code also violates TSM03-J. Do not publish partially initialized objects.

Compliant Solution (Volatile)

This compliant solution declares the helper field volatile:

// Works with acquire/release semantics for volatile
// Broken under JDK 1.4 and earlier
final class Foo {
  private volatile Helper helper = null;

  public Helper getHelper() {
    if (helper == null) {
      synchronized (this) {
        if (helper == null) {
          helper = new Helper();
        }
      }
    }
    return helper;
  }
}

When a thread initializes the Helper object, a happens-before relationship is established between this thread and any other thread that retrieves and returns the instance [Pugh 2004], [Manson 2004].

Compliant Solution (Static Initialization)

This compliant solution initializes the helper field in the declaration of the static variable [Manson 2006].

final class Foo {
  private static final Helper helper = new Helper();

  public static Helper getHelper() {
    return helper;
  }
}

Variables that are declared static and initialized at declaration or from a static initializer are guaranteed to be fully constructed before being made visible to other threads. However, this solution forgoes the benefits of lazy initialization.

Compliant Solution (Initialize-on-Demand, Holder Class Idiom)

This compliant solution uses the initialize-on-demand, holder class idiom that implicitly incorporates lazy initialization by declaring a static variable within a static Holder inner class:

final class Foo {
  // Lazy initialization
  private static class Holder {
    static Helper helper = new Helper();
  }

  public static Helper getInstance() {
    return Holder.helper;
  }
}

Initialization of the static helper field is deferred until the getInstance() method is called. The necessary happens-before relationships are created by the combination of the class loader's actions loading and initializing the Holder instance and the guarantees provided by the Java memory model (JMM). This idiom is a better choice than the double-checked locking idiom for lazily initializing static fields [Bloch 2008]. However, this idiom cannot be used to lazily initialize instance fields [Bloch 2001].

Compliant Solution (ThreadLocal Storage)

This compliant solution (originally suggested by Alexander Terekhov [Pugh 2004]) uses a ThreadLocal object to track whether each individual thread has participated in the synchronization that creates the needed happens-before relationships. Each thread stores a non-null value into its thread-local perThreadInstance only inside the synchronized createHelper() method; consequently, any thread that sees a null value must establish the necessary happens-before relationships by invoking createHelper().

final class Foo {
  private final ThreadLocal<Foo> perThreadInstance = 
      new ThreadLocal<Foo>();
  private Helper helper = null;

  public Helper getHelper() {
    if (perThreadInstance.get() == null) {
      createHelper();
    }
    return helper;
  }

  private synchronized void createHelper() {
    if (helper == null) {
      helper = new Helper();
    }
    // Any non-null value can be used as an argument to set()
    perThreadInstance.set(this);
  }
}

Noncompliant Code Example (Immutable)

In this noncompliant code example, the Helper class is made immutable by declaring its fields final. The JMM guarantees that immutable objects are fully constructed before they become visible to any other thread. The block synchronization in the getHelper() method guarantees that all threads that can see a non-null value of the helper field will also see the fully initialized Helper object.

public final class Helper {
  private final int n;
 
  public Helper(int n) {
    this.n = n;
  }
 
  // Other fields and methods, all fields are final
}
 
final class Foo {
  private Helper helper = null;
 
  public Helper getHelper() {
    if (helper == null) {            // First read of helper
      synchronized (this) {
        if (helper == null) {        // Second read of helper
          helper = new Helper(42);
        }
      }
    }
    return helper;                   // Third read of helper
  }
}

However, this code is not guaranteed to succeed on all Java Virtual Machine platforms because there is no happens-before relationship between the first read and third read of helper. Consequently, it is possible for the third read of helper to obtain a stale null value (perhaps because its value was cached or reordered by the compiler), causing the getHelper() method to return a null pointer.

Compliant Solution (Immutable)

This compliant solution uses a local variable to reduce the number of unsynchronized reads of the helper field to 1. As a result, if the read of helper yields a non-null value, it is cached in a local variable that is inaccessible to other threads and is safely returned.

public final class Helper {
  private final int n;
 
  public Helper(int n) {
    this.n = n;
  }
 
  // Other fields and methods, all fields are final
}
 
final class Foo {
  private Helper helper = null;
 
  public Helper getHelper() {
    Helper h = helper;       // Only unsynchronized read of helper
    if (h == null) {
      synchronized (this) {
        h = helper;          // In synchronized block, so this is safe
        if (h == null) {
          h = new Helper(42);
          helper = h;
        }
      }
    }
    return h;
  }
}

Exceptions

LCK10-J-EX0: Use of the noncompliant form of the double-checked locking idiom is permitted for 32-bit primitive values (for example, int or float) [Pugh 2004], although this usage is discouraged. The noncompliant form establishes the necessary happens-before relationship between threads that see an initialized version of the primitive value. The second happens-before relationship (for the initialization of the fields of the referent) is of no practical value because unsynchronized reads and writes of primitive values up to 32-bits are guaranteed to be atomic. Consequently, the noncompliant form establishes the only needed happens-before relationship in this case. Note, however, that the noncompliant form fails for long and double because unsynchronized reads or writes of 64-bit primitives lack a guarantee of atomicity and consequently require a second happens-before relationship to guarantee that all threads see only fully assigned 64-bit values (see  VNA05-J. Ensure atomicity when reading and writing 64-bit values for more information).

Risk Assessment

Using incorrect forms of the double-checked locking idiom can lead to synchronization problems and can expose partially initialized objects.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

LCK10-J

Low

Probable

Medium

P4

L3

Automated Detection

Tool
Version
Checker
Description
CodeSonar4.2FB.MT_CORRECTNESS.DC_DOUBLECHECKPossible double check of field
Coverity7.5

DOUBLE_CHECK_LOCK
FB.DC_DOUBLECHECK

Implemented
Parasoft Jtest 10.3 TRS.DCLImplemented
SonarQube
6.7
S2168

Related Guidelines

MITRE CWE

CWE-609, Double-checked Locking

Bibliography




27 Comments

  1. Another compliant solution suggested by Thomas Hawtin on the JMM mailing list:

    // Uses atomic utilities
    class Foo {
      private final AtomicReference<Helper> helperRef =
        new AtomicReference<Helper>();
    
      public Helper getHelper() {
        Helper helper = helperRef.get();
        if (helper != null) {
           return helper;
        }
        Helper newHelper = new Helper();
        return helperRef.compareAndSet(null, newHelper) ?
               newHelper :
               helperRef.get();
      }
    }
    
    1. This has one potential disadvantage. It does allow for multiple threads to construct Helper objects, and all such objects get GCed except the first. If multiple calls to 'new Helper()' is a problem, then you can't use this solution.

      Other than that, this solution works fine.

      1. Theoretically, it is possible that multiple threads may cause the memory to run out.

        1. True, but I wouldn't mention that here. First of all if you have enough threads that constructing a Helper in each one might exhaust memory, you've got bigger problems. Second, we haven't really given thought to memory exhaustion in scenarios that didn't involve recursion or infinite loops. That is, we have assumed that for anything that isn't a 'runaway process' there is enough memory to do whatever you want. (Maybe that isn't valid for embedded devices.)

          At any rate, I have no problem with the code as a CS, I just think we need to add the disclaimer that the solution is useless if you must only call the Helper constructor once.

          1. If the construction of Helper is expensive (and that's why we have this guideline in the first place; lazy initialization and so on), the OOM error is not inconceivable. This won't have been an issue if we were dealing with objects whose construction is fast and consumes less memory.

            I think your disclaimer point about the other issue is valid and I can imagine a class running afoul if it contains an instance field that gets a unique id from a static field (like what we did in CON12). This may/may not be the desired behavior.

  2. The CS (java.util.concurrent utilities) could possibly declare newHelper as a weak reference to mitigate the memory issue.

  3. Aleksey Shipilev @ oracle says, via email:

    One of my fellow readers (Andrei, CC'ed) pointed out the "Immutable" idiom listed in this rule is not really compliant. Namely, the second read here is allowed to return null, breaking the basic invariants for double-checked locking:

    final class Foo {
      private Helper helper = null;
    
      public Helper getHelper() {
        if (helper == null) { // first racy read
          synchronized (this) {
            if (helper == null) {
              helper = new Helper(42);
            }
          }
        }
        return helper;  // second racy read
      }
    }
    

    Specifically, this paragraph is wishful thinking: "Additionally, the block synchronization in the getHelper() method suffices to ensure that all methods that can see a non-null value of the helper field have a proper happens-before relationship for the update to the helper reference."

    Under JMM rules, the unsynchronized read of helper is still a data race, and there is NO HAPPENS-BEFORE RELATIONSHIP between the write of helper and the reads of it. That means, even though we are disallowed to see under-initialized Helper object once we see the non-null reference to it (due to "final" rules), it is still plausible to see the null reference to helper. Yes, that is plausible even if the first racy read got us the non-null one – JMM does not restrict this.

    See more details and experimental study at: http://shipilev.net/blog/2014/safe-public-construction/ http://jeremymanson.blogspot.ru/2008/12/benign-data-races-in-java.html

    Thank you,
    -Aleksey.

    1. My response:

      The CERT code sample you cite is analogous to the FinalWrapperFactory in your blog link: http://shipilev.net/blog/2014/safe-public-construction/. The link suggests that publication is safe.

      Remember for that example: Helper is immutable, with final fields. This guarantees that if helper is seen as non-null by the second thread to encounter the synchronized block, the second thread will see helper completely initialized. (The other samples do not assume that Helper is final.)

      1. David, I feel like my original note was ignored completely, let me try again. Please notice this is not about the safe construction/publication of helper itself, it is about the basic invariants of lazy initialization: the user should always get the non-null Helper reference from the getHelper() call. The CERT example allows getHelper() to return "null", obliterating that invariant.

        Please also notice the CERT code sample is NOT analogous to FinalWrapperFactory my article has. There is a critical distinction that I pointed out initially: CERT example does 2 racy reads, and risks reading "null" on the second read of helper. FinalWrapperFactory explicitly does only a single racy read, and recovers if it reads null. The entire endeavor with final Helper relies on the fact it survives the racy publication, but you have to exercise caution to not to cause more races that final can not protect you from. 

        This is not a theoretical issue, this happens in practice. If you re-read the ARM result here: http://shipilev.net/blog/2014/safe-public-construction/#_arm, and see the behavior of "Unsafe DCL" + "Safe Singleton", it will be almost exactly the CERT code sample we are discussing – two racy reads, with a safe final-bearing object, allows "null" as the result.

        1. Aleksey:

          Sorry, I got confused. Your blog post starts off discussing safe initialization & publication, which distracted me, because in the Immutable compliant solution, these are not issues. WRT safe initialization & publication, FinalWrapperFactory and the CERT code are analogous.

          The difference between FinalWrapperFactory and the Immutable CS is that the Immutable CS does not use a local helper variable. I presume the simplest way to fix the Immutable CS would be to create a local Helper variable and read from that, as you did in both FinalWrapperFactory and UnsafeLocalDCL, right?

          Under JMM rules, the unsynchronized read of helper is still a data race, and there is NO HAPPENS-BEFORE RELATIONSHIP between the write of helper and the reads of it.

          I agree that there is no happens-before between the write and the reads. And your benchmarks do suggest that the Immutablity CS can still fail.

          But I can still argue that the Immutable CS code is correct. Consider the definition of 'happens-before', from JLS 17.4.5:

          Two actions can be ordered by a happens-before relationship. If one action happens-before another, then the first is visible to and ordered before the second.

          If we have two actions x and y, we write hb(x, y) to indicate that x happens-before y.

          If x and y are actions of the same thread and x comes before y in program order, then hb(x, y).

          This suggests that if the first racy-read returns non-null, then the second racy-read should too.

          Concurrency brings lots of surprises to developers; we cover some counterintuitive examples in our Java introduction. Consider this (simpler) code example:

          int x = 0, y = 0, z = 0

          Thread 1

          Thread 2

          x = 1;

          y = x;

           

          z = x;

          With no happens-before relationship established between the threads, everyone acknowledges this code could complete with y and z both 0, or both 1, or y==0&&z==1. I have never believed that y==1&&z==0 was permissible, because happens-before on the two statements in Thread 2 would prevent that.

          One final note, from your blog:

          We see that a failure happens when an unsafely initialized singleton meets an unsafely publishing singleton factory. In all other cases, either safe publication or safe initialization saved us from breaking.

          This rule was violated by your SafeSingleton+UnsafeDCL example, where you report failure despite safe publication. That point could use some clarification in your blog.

          1. Hi David,

            I presume the simplest way to fix the Immutable CS would be to create a local Helper variable and read from that, as you did in both FinalWrapperFactory and UnsafeLocalDCL, right?

            Yes, that is a simplest fix. You have to do a single read, and then either recover if you have read null, or return the value. Doing the read second time sets you up for the data race.

             

            But I can still argue that the Immutable CS code is correct.

            No, it isn't, sorry. Your explanation relies on a common misinterpretation of JMM spec: the assumption that actions bound in happens-before order *consistently* observe the effects in that "global" order. They are not. In reality, the happens-before consistency rules are governing what particular write can a particular read see, for each read in isolation.

            Please read from here: http://shipilev.net/blog/2014/jmm-pragmatics/#_java_memory_model. In your example, the inter-thread consistency yields a very narrow class of executions, containing five actions (V1 and V2 stand for some template values we have actually read/written, this allows me to make the example shorter):

            write(x, 1); read(x, V1); write(y, V1); read(x, V2); write(z, V2)

            Even though you can argue there is a happens-before order like this:

            read(x, V1) --hb--> write(y, V1) --hb--> read(x, V2) --hb--> write(z, V2)

            ...you may variate V1 and V2, and see that neither value of {0, 1} is violating the happens-before consistency. For the reads and writes to the same location, the read is allowed to observe either the latest write in happens-before order, or any other write, that is not happens-before it (this is a race). In this example, both reads of "x" are the reads via the data races. In other words:
            - V1/V2 is allowed to be 0, because read(x, V1/V2) is allowed to observe the default write
            - V1/V2 is allowed to be 1, because read(x, V1/V2) is allowed to observe the unordered write(x, 1)

            Therefore, the outcome (y == 1) && (z == 0) is acceptable under JMM rules. If you want to disallow that, you need to declare "x" volatile, so that all actions on "x" become the synchronization actions, and get bound in total synchronization order. Then, synchronization order consistency will enforce that racy reads are not possible, and the only write you can see is the latest write to the same location. The corollary for that would be that two consecutive reads would see the same value.

            P.S. Thanks for the pointers on what to correct in my blog entry, I should attend to them as well (smile)

             

            1. I have implemented the change to the compliant solution that we agreed upon.
              I retained the old code as a noncompliant code example.

              1. Thanks, David. Your code has bug though, you have to re-read helper under synchronized (also, "private" for local variable definition is syntax error):

                 

                  public Helper getHelper() {
                    private Helper h = helper;       // only one racy read of helper
                    if (h == null) {
                      synchronized (this) {
                                  h = helper;      
                        if (h == null) {
                          h = new Helper(42);
                          helper = h;
                        }
                      }
                    }
                    return h;
                  }
                P.S. I have updated my blog post as well to make it more clear the discussed race is important.
          2. David Svoboda While, I agree with the Aleksey's reply to your comment, I am uncertain whether the example you have given applies to this situation. The example you have given seems misleading to me because it states that one thread writes variable x and another reads that variable which is not what is happening in this double-checked locking idiom.

            What we have actually is more like this:

            Thread1Thread2
            read(x)read(x)
            write(x, 42)read(x)
            read(x)


            We don't expect that 2nd read in the 1st Thread will return anything other then 42, do we? Of course, we cannot have any guarantee of what any of the reads in the 2nd Thread would return.

            To quote the JLS again: If x and y are actions of the same thread and x comes before y in program order, then hb(x, y).
            I would argue that in a situation where thread writes to the helper variable there must be a happens before relationship between write to the helper  variable and third read of the helper variable because the same thread writes to the variable and reads that variable.
            To sum up, if a thread assigned a value to the helper this thread cannot return null as the helper value because this would violate the JLS specification. I think we must agree there is a happens-before relationship between write to the helper variable and a third read in this case.

            Additionally, I believe that entering synchronized block has a read-acquire semantics in Java which means any reads/writes that occur after it are not allowed to be reordered to occur before entering synchronized block.

            Having thought about all the possibilities, from my perspective it is obvious that if 1st read returns null, thread will enter the first if section and acquire a lock which must happen before the third read.  Acquiring a lock assures us that 3rd read has a consistent value.
            However, if 1st read returns non-null value I don't see a guarantee that 3rd read must also return a non-null value as well. It seems possible that 3rd read was reordered to occur before 1st read in which case 3rd read could get a null value after which 1st read gets a non-null value. In this scenario thread will not enter the if block or acquire a lock which in turn doesn't enforce happens-before relationship between 1st and 3rd read.
            I am uncertain if assembler code can be made that would make both situations possible. Either the 1st read happens before the 3rd read or it doesn't, how can we have both? And I believe I've presented a case in which 1st read is guaranteed to happen before 3rd read.

            I don't know how synchronization gets along with if-else conditionals and I think this issue requires a more appropriate explanation.

  4. I have a slightly different requirement.  It involves lazy loading values in a map for a given key.  Hence the if conditions are on the values in the map.

    Consider the code below.  The premise is that it is safe to do double check locking in this case, as a thread doing the first if condition check would always only see a fully initialized value for helper.

    Is this a correct understanding?

    public class Foo {
    
        private Map<String, Helper> helpersMap = new HashMap<>();
    
        public Helper getInstanceFor(String key) {
    
            Helper helper = helpersMap.get(key);
            if (helper == null) {
                synchronized (this) {
                    if (helper == null) {
                         helper = new Helper();
                         helpersMap.put(key, helper); // Map.put() would always receive a fully initialized value of helper
                    }
                }
            }
    
            return helper;
        }
    }
    
    
    
    1. Incorrect. Your code example suffers from the same vulnerability as the first noncompliant code example. It is possible for the compiler or the processor to reorder the statements so that a thread that calls getInstanceFor() gets a non-null Helper object, but that object is only partially constructed (the 'new Helper()' is invoked in a different thread, and is not finished yet).

      You can mitigate this problem by making the getInstanceFor() method synchronized, which will not return a Helper object to a different thread until the Helper constructor is complete. But a better option would be to use a ConcurrentHashMap.

      1. Thanks for your prompt reply David Svoboda

        The different between the first non-compliant one and the one stated above is that "helper" local method variable. 

        To demonstrate why I suspect this is not an issue, see the revised code sample below where there is no "helper" variable" outside the synchronized block, but the code basically does the same thing as the one above.

        Also consider that the methods Map.get() and Map.put() are methods which for arguments sake could have been any kind of methods having a whole bunch of logic behind it.  It seems scary to think that compiler would restructure things which result in calls to them with objects in invalid state

        public class Foo {
         
            private Map<String, Helper> helpersMap = new HashMap<>();
         
            public Helper getInstanceFor(String key) {
         
                if (helpersMap.get(key) == null) { // Could use Map.containsKey() as well
        
                    synchronized (this) {
                        if (helpersMap.get(key) == null) { // Could use Map.containsKey() as well
        
                             Helper helper = new Helper(); 
                             helpersMap.put(key, helper); // Map.put() would always receive a fully initialized value of helper
                        }
                    }
                }
         
                return helpersMap.get(key);
            }
        }

        Does this still suffer from the problem?  

        1. Ranjan:
          Sorry, your newer code example suffers from the same problem. The fact that helper was a local variable was not the problem with your first code example...since it is a local variable, it is invisible to any threads except the one whose stack it lives on.

          The problem in both code examples is that the call to helpersMap.get(key) returns an object that is accessible by multiple threads. When one thread initializes it, using helperMap.put(...), the helpersMap is updated. A second thread that calls getInstanceFor() with the same key might see the helpersMap.get(key) call is no longer null, but might contain a Helper object whose constructor has not completed.

          This is explained in greater depth in TSM03-J. Do not publish partially initialized objects

          1. David Svoboda

            The heart of the confusion in my mind lies in the underlined statement of the following line:

            "second thread that calls getInstanceFor() with the same key might see the helpersMap.get(key) call is no longer null, but might contain a Helper object whose constructor has not completed."

            So in the following lines of code within the synchronized block shown above:

            Helper helper = new Helper();
            helpersMap.put(key, helper); // Map.put() would always receive a fully initialized value of helper

            ... is it the case that the thread within the synchronized block is allowed to pass on to the "put()" method a reference to "helper" before its initialization is complete?

            That sounds spurious to me even in a single threaded model.  Assuming "put()" is just any other method and not necessarily a map, what if it were to call a method on the helper passed in?

            Isn't it guaranteed that "put()" would only be called after the initialization is complete?

            ==

            The code above is very similar to the "Compliant Solution (Final Field and Thread-Safe Composition)" in TSM03-J. Do not publish partially initialized objects, which is also doing a double check using the "isEmpty()" method call with the following key differences:

            • My example has a map while that one has a Vector
            • The field equivalent to "helpersMap" is final to ensure it is initialized and not null when threads call getInstanceForKey() (which I can do in my situation as well) 
            • The collection used in the example there is a thread-safe Vector.  On this front we would just get additional threads attempting to enter the synchronized blocks, but they would exit immediately due to the double check done inside, if a previous thread already entered

            Regardless it still feels like threads would not see partially built "Helper" instances in the map in the above code.


            1. Ranjan

              This is a common stumbling block, and one of the things that makes concurrency Very Hard to Get Right(tm).

              It might help if you stop thinking of the JVM as your faithful interpreter, and instead think of it as a slave to a written specification (the Java Language Specification), that imposes some constraints on it, but gives it freedom in some surprising ways. It uses that freedom to make itself faster, but sometimes that freedom has weird side effects, most often in multithreaded code.

              The biggest constraint imposed on it in multithreaded code is the happens-before relationship, which requires an event in one thread to see the results of a prior event in another thread. For concurrency safety, we want the initialization of Helper in the thread initializing it to be complete and visible to a second thread. This isn't specified by the JLS, but it provides mechanisms that can guarantee the relationship...the synchronized keyword, for example.

              One thing the JLS requires is that within one thread, any two statements have a happens-before relationship. So if you examined the Helper object before calling Map.put(), you would see a fully-initialized Helper object. (Map.put() doesn't examine Helper, so it might get, or need, a fully-initialized object)

              1. David Svoboda - Thanks for your patience and detailed explanation, was on vacation.

                So just to round of my understanding and hopefully a last question for clarity, based on the following:

                One thing the JLS requires is that within one thread, any two statements have a happens-before relationship. So if you examined the Helper object before calling Map.put(), you would see a fully-initialized Helper object. (Map.put() doesn't examine Helper, so it might get, or need, a fully-initialized object)

                Theoretically, if I had a read on the helper object just after construction in the synchronization, something like the following, the map would always receive a fully constructed Helper object?

                Helper helper = new Helper();
                
                helper.someMethod(); // This would cause the required examination
                
                helpersMap.put(key, helper); // Map.put() would always receive a fully initialized value of helper

                I do understand that there is a danger that future modification of this code by some developer may remove this read for whatever reason and we would be back to the problem at hand.

                1. Ranjan:

                  This *might* solve your partially-initialized-object problem, but is not guaranteed to.

                  Your theory is plausible that calling someMethod() on helper makes it more likely to be fully initialized when added to the map...depending on what someMethod() does.  It is also plausible that if both threads look at the same memory containing helper that this makes the outside thread see a fully-initialized object.

                  But the JLS does not require this. For example, it is possible for two threads to run on two different processors, each with a private cache of memory. So while Thread 1 constructs helper, and before it finishes initializing helper, it gets copied to Thread 2's cache. So while Thread 1 adds helper to the map, Thread 2 reads the map and still sees a stale partially-initialized Helper object from its cache.

                  This is why you need a happens-before relationship between the two threads...to ensure that not only do events happen in the correct order, but to make sure that events in one thread (such as initialization) are visible to events in another thread.

                  1. Thank you David Svoboda.  Much appreciated!

            2. WRT your last question, java.util.Vector is magical; it imposes a concurrency constraint upon its contained objects that Maps do not. That's why the final compliant solution in TSM03-J is OK.

  5. Referred SonarQube rule for detecting this issue does not look correct. I guess it should be: squid:S2168 https://www.sonarsource.com/products/codeanalyzers/sonarjava/rules.html#RSPEC-2276