Java - Preferred design for using a mutable object reference in another thread?

Question

public class ObjectA {

   private void foo() {
       MutableObject mo = new MutableObject();
       Runnable objectB = new ObjectB(mo);
       new Thread(objectB).start();
   }

}

public class ObjectB implements Runnable {

    private MutableObject mo;

    public ObjectB(MutableObject mo) {
        this.mo = mo;
    }

    public void run() {
       //read some field from mo
    }
}

As you can see from the code sample above, I pass a mutable object to a class that implements Runnable and will use the mutable object in another thread. This is dangerous because ObjectA.foo() can still alter the mutable object's state after starting the new thread. What is the preferred way to ensure thread safety here? Should I make copy of the MutableObject when passing it to ObjectB? Should the mutable object ensure proper synchronization internally? I've come across this many times before, especially when trying to use SwingWorker in a number of GUI applications. I usually try to make sure that ONLY immutable object references are passed to a class that will use them in another thread, but sometimes this can be difficult.

Answer 1

This is a hard question, and the answer, unfortunately, is 'it depends'. You have three choices when it comes to thread-safety of your class:

1. Make it Immutable, then you don't have to worry. But this isn't what you're asking.
2. Make it thread-safe. That is, provide enough concurrency control internal to the class that client code doesn't have to worry about concurrent threads modifying the object.
3. Make it not-thread safe, and force client code to have some kind of external synchronization.

You're essentially asking whether you should use #2 or #3. You are worried about the case where another developer uses the class and doesn't know that it requires external synchronization. I like using the JCIP annotations @ThreadSafe @Immutable @NotThreadSafe as a way to document the concurrency intentions. This isn't bullet-proof, as developers still have to read the documentation, but if everyone on the team understands these annotations and consistently applies them, it does make things clearer.

For your example, if you want to make the class not thread-safe, you could use AtomicReference to make it clear and provide synchronization.

public class ObjectA {

  private void foo() {
     MutableObject mo = new MutableObject();
     Runnable objectB = new ObjectB(new AtomicReference<>( mo ) );
     new Thread(objectB).start();
  }
}

public class ObjectB implements Runnable {

  private AtomicReference<MutableObject> mo;

  public ObjectB(AtomicReference<MutableObject> mo) {
    this.mo = mo;
  }

  public void run() {
   //read some field from mo
   mo.get().readSomeField();
  }
}

Answer 2

I think you are overcomplicating it. If it is as the example (a local variable of which no reference is kept) you should trust that nobody will try to write to it. If it is more complicated (A.foo() has more LOC) if possible, create it only to pass to the thread.

new Thread(new MutableObject()).start();

If not (due to initializations), declare it in a block so it gets out of scope immediately, even maybe in a separate private method.

{
   MutableObject mo = new MutableObject();    
   Runnable objectB = new ObjectB(mo);    
   new Thread(objectB).start();    
}
....

Answer 3

Copy the object. You won't have any weird visibility problems because you pass the copy to a new Thread. Thread.start always happens before the new thread enters its run method. If you change this code to pass the object to an existing thread, you need proper synchronization. I recommend a blocking queue from Java.util.concurrent.

Answer 4

Without knowing your exact situation, this question will be difficult to answer precisely. The answer totally depends on what the MutableObject represents, how many other threads may modify it simultaneously, and whether or not the threads that read the object care whether its state changes while they are reading it.

With respect to thread-safety, internally synchronizing all reads and writes to MutableObject is provably the "safest" thing to do, but it comes at the cost of performance. If contention is really high on reads and writes, then your program may suffer performance issues. You can get better performance by sacrificing some guarantees on mutual exclusion - whether those sacrifices are worth the performance increases totally depends on the specific problem you're trying to solve.

You can also play some games with how you go about "internally synchronizing" your MutableObject, if that's what you end up doing. If you haven't already, I'd recommend reading up on the differences between volatile and synchronized and understand how each can be used to ensure thread safety for different situations.