Why use both local variable and volatile both togather for DCL (as suggested by Sonar tool)?

120 Views Asked by At

There are related questions but they as the focus in those is more on volatile and not much on the usage of local variable, hence I am posting a new question, Please see the question below.

Sonar Source rule has a code according to which below code is correct and solves the issues with DCL

class ResourceFactory {
  private volatile Resource resource;

  public Resource getResource() {
    Resource localResource = resource;
    if (localResource == null) {
      synchronized (this) {
        localResource = resource;
        if (localResource == null) {
          resource = localResource = new Resource();
        }
      }
    }
    return localResource;
  }

  static class Resource {
  }
}

I understand that we have alternate options (some better than complicated DCL) like an

  1. enum or
  2. static inner class holder or
  3. a static final reference initialized during its declaration itself.
  4. declare the entire method as synchronized.

I wish to understand the usage of volatile along with local variable to resolve the issues for DCL.

Also I understand volatile (for Java 1.5 and later versions) guarantees the 'happens-before' feature which prevents the in-complete object being returned for reading.

Question 1 I want to understand as what is the need of volatile if a local variable has already been used. Below code has non volatile variable resource but inside the getResource() a local variable is used to read the resource. Are there still issues in the code ? (Please see comments in below code below)

class ResourceFactory {
  private Resource resource; // non volatile
  public Resource getResource() {
    Resource localResource = resource; // only 1 read without lock
    if (localResource == null) {
      synchronized (this) {
        localResource = resource; // read inside the synchronized block
        if (localResource == null) {
           localResource = new Resource(); // constructor is initialized. 
           resource = localResource; // ****code reaches here only when constructor has been complete. Here is there a chance of resource being referring to incomplete object ?**** 
        }
      }
    }
    return localResource;
  }

  static class Resource {
  }
}

The only advantage I see is with volatile is suppose the object was created then the first read itself would have guaranteed a memory read instead of read from thread cache preventing unnecessary acquisition of lock.

Question 2 Instead of using local variable if simple volatile was used then was the code without issues ?

1

There are 1 best solutions below

1
Andy Turner On

Question 2 is directly addressed in Effective Java; whilst it may be mentioned in other editions too, the one I have in front of me is 3rd Ed, Item 83, p335:

The code may appear a bit convoluted. In particular, the need for the local variable (result) may be unclear. What this variable does is to ensure that field is read only once in the common case where it's already initialized. While not strictly necessary, this may improve performance and is more elegant by the standards applied to low-level concurrent programming. On my machine, the method above is about 1.4 times as fast as the obvious version without a local variable.

(Note that 3rd Ed has a serious error in the code sample for double-checked locking (immediately preceding this quote) in early printings! Check out the errata for corrected code.)