Private constructor to avoid race condition

user_1389813 0
Viewed 4346

I am reading the book Java Concurrency in Practice session 4.3.5

  @ThreadSafe
  public class SafePoint{

       @GuardedBy("this") private int x,y;

       private SafePoint (int [] a) { this (a[0], a[1]); }

       public SafePoint(SafePoint p) { this (p.get()); }

       public SafePoint(int x, int y){
            this.x = x;
            this.y = y;
       }

       public synchronized int[] get(){
            return new int[] {x,y};
       }

       public synchronized void set(int x, int y){
            this.x = x;
            this.y = y;
       }

  }

I am not clear where It says

The private constructor exists to avoid the race condition that would occur if the copy constructor were implemented as this (p.x, p.y); this is an example of the private constructor capture idiom (Bloch and Gafter, 2005).

I understand that it provides a getter to retrieve both x and y at once in a array instead of a separate getter for each, so the caller will see consistent value, but why private constructor ? what's the trick here

race-condition multithreading java
9 Answers

What does it mean is, if you did not have a private constructor and you implement copy constructor in following way:

public SafePoint(SafePoint p) {
    this(p.x, p.y);
}

Now assume that thread A is having the access to SafePoint p is executing above copy constructor's this(p.x, p.y) instruction and at the unlucky timing another thread B also having access to SafePoint p executes setter set(int x, int y) on SafePoint p. Since your copy constructor is accessing p's x and y instance variable directly without proper locking it could see inconsistent state of SafePoint p.

Where as the private constructor is accessing p's variables x and y through getter which is synchronized so you are guaranteed to see consistent state of SafePoint p.

user_86399520

Our requirement is: we want to have a copy constructor like below (at the same time ensuring class is still thread safe):

public SafePoint(SafePoint p){
    // clones 'p' passed a parameter and return a new SafePoint object.
}

Let's try to make the copy constructor then.

Approach 1:

public SafePoint(SafePoint p){
    this(p.x, p.y);
}

The problem with above approach is that it will render our class NOT THREAD SAFE

How ?

Because constructor is NOT synchronised, meaning it is possible that two threads can simultaneously act on the same object (one thread might clone this object using it's copy constructor and other thread might invoke object's setter method). And if this happens, the threads that invoked the setter method could have updated the x field (and yet to update the y field) thereby rendering the object in an inconsistent state. Now at this point, if the other thread (which was cloning the object), executes (and it can execute because constructor is not synchronised by intrinsic lock) the copy constructor this(p.x, p.y), p.x would be the new value, while p.y would still be old.

So, our approach is not thread safe, because constructor is not synchronised.

Approach 2: (Trying to make approach 1 thread safe)

public SafePoint(SafePoint p){
    int[] temp = p.get();
    this(temp[0], temp[1]);
}

This is thread safe, because p.get() is synchronised by intrinsic lock. Thus while p.get() executes, other thread could not execute the setter because both getter and setter are guarded by the same intrinsic lock.

But unfortunately the compiler won't allow us do this because this(p.x, p.y) should be the first statement.

This brings us to our final approach.

Approach 3: (solving compilation issue of approach 2)

public SafePoint(SafePoint p){
    this(p.get());
}

private SafePoint(int[] a){
    this(a[0], a[1]);
}

With this approach, we are guaranteed that our class is thread safe and we have our copy constructor.

One final question remaining is why is the second constructor private ? This is simply because we create this constructor just for our internal purpose and we don't want client to create SafePoint object by invoking this method.

user_52559350
Related