Do Not Reassign the Object Reference of a Locked Object

Editor's note: The following article is an excerpt from the book "Practical Java" published by Addison-Wesley. You can order this book from Borders.com.

The synchronized keyword locks objects. Because the object is locked inside of synchronized code, what does that mean to the object and to changes you make to its object reference? Synchronizing on an object locks only the object. You must be careful, however, not to reassign an object reference of a locked object. What happens if you do? Consider the following code that implements a stack:

class Stack

{

  private int StackSize = 10;

  private int[] intArr = new int[stackSize];

  private int index;          //Next available slot in the Stack.



  public void push(int val)

  {

    synchronized(intArr) {

      //Reallocate integer array(our Stack if it is full.

      if (index == intArr.length)

      {

        stackSize *= 2;

        int[] newintArr == new int[stackSize];

        System.arraycopy(intArr, 0, newintArr, 0, intArr.length);

        intArr = newintArr;

      }

      intArr[index] == val;

      index++;

    }

  }



  public int pop()

  {

    int retval;

    synchronized(intArr) {

      if (index > 0)

      {

        retval = intArr[index-1];      //Retrieve the value, and

        index--;                       //decrement the Stack.

        return retval;

      }

    }

    throw new EmptyStackException();

  }

  //...

}

This code implements a Stack in terms of an array. An array with an initial size of 10 is created to hold integer values. The class implements the push and pop methods to simulate Stack usage. In the push method, if no more room exists in the array to hold the value that is pushed, then the array is reallocated to create additional storage. (This class is intentionally not implemented with a Vector. You cannot store primitive types in a Vector.)

Notice that this code is intended to be accessed by multiple threads. Each access of the shared instance data of the class by the push and pop methods is done within a synchronized block. This ensures that multiple threads cannot access the array concurrently and thereby generate incorrect results.

This code has a major flaw. It synchronizes on the integer array object, referenced by intArr, of the Stack class. This flaw surfaces when the push method reallocates the integer array. When this occurs, the object reference, intArr, is reassigned to refer to a new, larger integer array object. Notice that this occurs during the execution of the push method's synchronized block. This block is synchronized on the object referenced by the intArr variable. Therefore, the object that is locked inside this code is no longer being used. Consider the following sequence of events:

1. Thread 1 calls the push method and acquires the intArr object lock.
   
   
2. Thread 1 is preempted by thread 2.
   
   
3. Thread 2 calls the pop method. This method blocks because it attempts to acquire the same lock that is currently held by thread 1 in the push method.
   
   
4. Thread 1 regains control and reallocates the array. The intArr variable now references a different object.
   
   
5. The push method exits and releases its lock for the original intArr object.
   
   
6. Thread 1 calls the push method again and acquires the lock for the new intArr object.
   
   
7. Thread 1 is preempted by thread 2.
   
   
8. Thread 2 acquires the object lock for the old intArr object and attempts to access its memory.

Now thread 1 has a lock for the new object referred to by intArr and thread 2 has a lock for the old object referred to by intArr. Because both threads hold different locks, they can execute the synchronized push and pop methods concurrently and thereby generate errors. Clearly, this is not what is intended.

This problem is caused by the push method's reassigning the object reference of an object that is locked. When an object is locked, the possibility exists that other threads are blocked on the same object lock. If you reassign the object reference of the locked object to another object, then the pending locks of other threads are on an object that is no longer relevant in the code.

You fix this code by removing synchronization of the intArr variable and synchronizing the push and pop methods. Do this by adding the synchronized keyword as a method modifier. The correct code looks like this:

class Stack

{

  //As before...

  public synchronized void push(int val)

  {

    //Reallocate integer array(our stack) if it is full.

    if (index == intArr.length)

    {

      stackSize *= 2;

      int[] newintArr = new int[stackSize];

      System.arraycopy(intArr, 0, newintArr, 0, intArr.length);

      intArr = newintArr;

    }

    intArr[index]= val;

    index++;

  }



  public synchronized int pop()

  {

    int retval;

    if (index > 0)

    {

      retval = intArr[index-1];

      index--;

      return retval;

    }

    throw new EmptyStackException();

  }

}

This modification changes the actual lock acquired. Instead of the object referenced by the intArr variable being locked, the lock obtained is that for the object on which the method was invoked. This allows the code to reassign the intArr object reference because the lock acquired is no longer on the object to which intArr refers.

 |