How do you ensure multiple threads can safely access a class field?

Question

When a class field is accessed via a getter method by multiple threads, how do you maintain thread safety? Is the synchronized keyword sufficient?

Is this safe:

public class SomeClass {
    private int val;

    public synchronized int getVal() {
        return val;
    }

    private void setVal(int val) {
        this.val = val;
    }
}

or does the setter introduce further complications?

Answer 1

From my understanding you should use synchronized on both the getter and the setter methods, and that is sufficient.

Edit: Here is a link to some more information on synchronization and what not.

Answer 2

If you use 'synchronized' on the setter here too, this code is threadsafe. However it may not be sufficiently granular; if you have 20 getters and setters and they're all synchronized, you may be creating a synchronization bottleneck.

In this specific instance, with a single int variable, then eliminating the 'synchronized' and marking the int field 'volatile' will also ensure visibility (each thread will see the latest value of 'val' when calling the getter) but it may not be synchronized enough for your needs. For example, expecting

 int old = someThing.getVal();
 if (old == 1) {
    someThing.setVal(2);
 }

to set val to 2 if and only if it's already 1 is incorrect. For this you need an external lock, or some atomic compare-and-set method.

I strongly suggest you read Java Concurrency In Practice by Brian Goetz et al, it has the best coverage of Java's concurrency constructs.

Answer 3

Synchronization exists to protect against thread interference and memory consistency errors. By synchronizing on the getVal(), the code is guaranteeing that other synchronized methods on SomeClass do not also execute at the same time. Since there are no other synchronized methods, it isn't providing much value. Also note that reads and writes on primitives have atomic access. That means with careful programming, one doesn't need to synchronize the access to the field.

Read Sychronization.

Not really sure why this was dropped to -3. I'm simply summarizing what the Synchronization tutorial from Sun says (as well as my own experience).

Using simple atomic variable access is more efficient than accessing these variables through synchronized code, but requires more care by the programmer to avoid memory consistency errors. Whether the extra effort is worthwhile depends on the size and complexity of the application.

Answer 4

In addition to Cowan's comment, you could do the following for a compare and store:

synchronized(someThing) {
    int old = someThing.getVal();
    if (old == 1) {
        someThing.setVal(2);
    }
}

This works because the lock defined via a synchronized method is implicitly the same as the object's lock (see java language spec).

Answer 5

For simple objects this may suffice. In most cases you should avoid the synchronized keyword because you may run into a synchronization deadlock.

Example:

public class SomeClass {

  private Object mutex = new Object();
  private int    val   = -1; // TODO: Adjust initialization to a reasonable start
                             //       value
  public int getVal() {
    synchronized ( mutex ) {
      return val;
    }
  }

  private void setVal( int val ) {
    synchronized ( mutex ) {
      this.val = val;
    }
  }
}

Assures that only one thread reads or writes to the local instance member.

Read the book "Concurrent Programming in Java(tm): Design Principles and Patterns (Java (Addison-Wesley))", maybe http://java.sun.com/docs/books/tutorial/essential/concurrency/index.html is also helpful...

Answer 6

If your class contains just one variable, then another way of achieving thread-safety is to use the existing AtomicInteger object.

public class ThreadSafeSomeClass {

    private final AtomicInteger value = new AtomicInteger(0);

    public void setValue(int x){
         value.set(x);
    }

    public int getValue(){
         return value.get();
    }

}

However, if you add additional variables such that they are dependent (state of one variable depends upon the state of another), then AtomicInteger won't work.

Echoing the suggestion to read "Java Concurrency in Practice".