How do I deep copy an irregular 2D array threadsafely

Question

I have stumbled upon an annoyance as I was writing a Java class; I couldn't figure out how to make copying a 2 dimensional array thread safe. This is the simple version of the class:

public class Frame {
  private boolean[][] content;

  public Frame(boolean [][] content) {
    boolean[][] threadSafeCopy = deepCopy(content);
    if (!(threadSafeCopy != null)) {
      throw new NullPointerException("content must not be null");
    }
    if (!(threadSafeCopy.length > 0)) {
      throw new IllegalArgumentException("content.length must be greater than 0");
    }
    if (!(threadSafeCopy[0] != null)) {
      throw new IllegalArgumentException("content[0] must not be null");
    }
    if (!(threadSafeCopy[0].length > 0)) {
      throw new IllegalArgumentException("content[0].length must be greater than 0");
    }
    for (int i = 1, count = threadSafeCopy.length; i < count; ++i) {
      if (!(threadSafeCopy[i].length == threadSafeCopy[0].length)) {
        throw new IllegalArgumentException
            (   "content[" + i + "].length [" + threadSafeCopy[i].length
              + "] must be equal to content[0].length [" + threadSafeCopy[0].length + "]"
            );
      }
    }
    this.content = threadSafeCopy;
  }

  private boolean[][] deepCopy(boolean[][] content) {
    boolean[][] result = null;
    if (content != null) {
      synchronized(content) { //do our best to make this as multi-threaded friendly as possible
        result = new boolean[content.length][];
        for (int i = 0, count = content.length; i < count; ++ i) {
          if (content[i] != null)
          {
            synchronized(content[i]) {
              result[i] = content[i].clone();
            }
          }
        }
      }
    }
    return result;
  }

  public boolean[][] getContent() {
    boolean[][] result = new boolean[this.content.length][]; //defensive copy
    for (int i = 0, count = result.length; i < count; ++i) {
      result[i] = this.content[i].clone(); //defensive copy
    }
    return result;
  }
}

However, the above implementation for the method private boolean[][] deepCopy(boolean[][] content) isn't actually threadsafe. It's possible that the array is being actively modified by another thread while this method is attempting the copy. Granted, I have guarded against the most abusive case, using synchronized on the base array. However, that doesn't cause the set of 2nd dimension array instances to be locked. And it's possible for them to be modified during the copy.

Is there is some way to collect the Object lock for each of the base array (content) and the sub-arrays (content[0], content[1], ..., content[content.length - 1]) such that I can call something like synchronized(objectsToLockSimultaneouslyList) and it lock all the objects simultaneously and in the list's order. If so, I can thread safely copy the contents of the array.

If not, what other kinds of solutions are available to "block all modifications to the array" without having to go change the classes which instantiate Frame or altering Frame's constructor such that it won't take an array, but only instances of immutable collections (which itself is down another freakin rabbit hole).

Thank you for any guidance you have in this area.

UPDATE: What I want to do is fundamentally not possible. And my understanding of locking objects via synchronized was also erred (tyvm glowcoder, Paulo and Brian). I am now going to attempt to change the interface on Frame to use List<List<Boolean>> which sure seems like it would be SO much more inefficient. Or I may use Set<XyCoordinate> where the presence of an XyCoordinate means "true". Again, that seems so freakin inefficient, but threadsafe. Ugh!

Answer 1

I think your best bet is to wrap the array in an object and provide synchronized access to it. Then you can do the deep copy with the "door shut" to the accessor method.

void deepCopy(boolean[][] orig) {
    synchronized(orig) {
        boolean[][] result = new boolean[orig.length][];
        deepCopy(orig,0);
        return result;        
    }
}

/**
 * recursive method to lock all rows in an array, and then copy
 * them in (backwards, by chance)
 */
void deepCopy(boolean[][] orig, int row) {
    if(row == orig.length) return; // end condition
    synchronized(orig[row]) { // lock the row first
        deepCopy(orig,row+1); // lock the next row
        content[row] = new boolean[orig[row].length];
        for(int i = 0; i < content[row].length; i++)
            content[row][i] = orig[row][i];
        // now, do row - 1
    }
}

Answer 2

You seem to have some misunderstanding about object locks.

A synchronized block (or method) on some object does not avoid any modifications on this object. It only avoids that other threads at the same time are in synchronized blocks or methods of the same object (with the exception on ones in wait() of the same object).

So, to be thread-safe here, you have to make sure all your accesses on the array synchronize on the same lock object.

Answer 3

I'd recommend that you look into using the java.util.concurrent package. It has a bunch of useful classes that might help you.

For your specific example, you can write a simple ThreadSafe2DArray class:

public class ThreadSafe2DArray {
    private ReadWriteLock readWriteLock;
    private Lock readLock;
    private Lock writeLock;
    private boolean[][] data;

    public ThreadSafe2DArray(int rows, int cols) {
        this.data = new boolean[rows][cols];
        this.readWriteLock = new ReentrantReadWriteLock();
        this.readLock = readWriteLock.readLock();
        this.writeLock = readWriteLock.writeLock();
    }

    public boolean get(int row, int col) {
        try {
            readLock.lock();
            return data[row][col];
        }
        finally {
            readLock.unlock();
        }
    }

    public void set(int row, int col, boolean b) {
        try {
            writeLock.lock();
            data[row][col] = b;
        }
        finally {
            writeLock.unlock();
        }
    }

    public boolean[][] copyData() {
        try {
            readLock.lock();
            // Do the actual copy
            ...
        }
        finally {
            readLock.unlock();
        }
    }
}

You can also expose the raw data array and the read/write locks if you want to allow clients to do their own locking, for example if clients could be reading or updating large numbers of entries in one chunk.

This approach is a lot more flexible than using raw synchronized blocks and may be faster or better for your case, although a lot depends on what sort of contention you expect in practice. The added benefit of this is that you don't need to force anything to be immutable.

Fundamentally, if two threads are ever going to be contending for a resource they must agree on some kind of locking mechanism. If the thread that populates the data array does not play nicely then the only option is to have a wrapper that forces locking. If the other thread is well-behaved then you could also synchronize on the top-level array object in both threads, or you can use the expose-locks-and-data approach, or just use the class as presented above and call copyData in your Frame class.

This answer got a lot longer than I intended. Hopefully it makes sense.

EDIT: Note that it's important to put the unlock call inside a finally block in case an unexpected RuntimeException (or an Error) gets thrown somewhere inside the try. In this case the code is pretty simple so it's probably overkill, but it will make the code more maintainable if this safety net is already in place.

EDIT2: I note that you specifically mentioned an irregular array. I've left the example as a regular array in the interests of brevity :)

Answer 4

I really think you're not grasping some core concepts of multithreading.

If some other thread has a reference to the array you're passing into the Frame constructor, nothing you do in Frame can help you. That other thread can modify it at will.