multiple fields: volatile or AtomicReference?

Question

I have to synchronize access between threads to a shared object, whose state consists of several fields. Say:

class Shared{
String a; Integer b;
//constructor, getters and setters
....
}

I have possibly many threads reading this objects, doing

//readers
shared.getA();
shared.getB();

and only one thread that will write at a certain point:

//writer
shared.setA("state");
shared.setB(1);

now my question is how to ensure that reading threads won't find the shared object in an inconsistent state.

I read many answers saying that for consistency between threads volatile is the solution,but I'm not sure how it works on multiple fields. E.g., is that enough?

volatile  String a; volatile Integer b;

Another solution would be to make the shared object immutable and use AtomicReference, E.g.,

AtomicReference<Shared> shared = ....

and then the writer will just swap the reference:

Shared prev = shared.get(); 
Shared newValue = new Shared("state",1);
while (!shared.compareAndSet(prev, newValue)) 

Is that approach correct? thanks!

Update In my setting the Shared objects are retrieved from a ConcurrentHashMap<Id,Shared>, so the comments agree that the way to go is either using the immutable approach or via synchronizing the updates on shared all together. However, for completeness would be nice to know whether the solution above with the ConcurrentHashMap<Id,AtomicReference<Shared>> is viable or wrong or just superfluous. Anyone can explain? thanks!

Answer 1

First of all you should make Shared immutable:

class Shared{
   private final String a; 
   private final int b;
   //constructor, getters and NO setters
}

And if you have only one writer you can safely use volatile, there is no need in AtomicRefference. At the point where information is updated old object should not be modified, but rather a new created and assigned to a volatile refference.

Answer 2

If you need to write both A and B together to keep them consistent, e.g. they are a name and a Social Security Number, one approach is to use synchronized everywhere and write a single, combined setter.

public synchronized void setNameAndSSN(String name, int ssn) {
   // do validation checking etc...
   this.name = name;
   this.ssn = ssn;
}

public synchronized String getName() { return this.name; }

public synchronized int getSSN() { return this.ssn; }

Otherwise the reader could "see" the object with the new name but with the old SSN.

The immutable approach makes sense too.

Answer 3

Marking fields volatile or making methods synchronised is not going to ensure atomicity.

Writer should take care of the atomicity.

Writer should call all the setters(that should be updated atomically) inside a synchronized block. synchronized(shared) { shared.setA() shared.setB() ... }

For this to work all the getters in the shared object should be synchronized too.

Answer 4

As @Mikhail says in his answer, making Shared immutable and replacing the whole object is a nice approach. If you don't want to or "can't" use that approach for some reason, you can just make sure all the fields on Shared are protected by the same lock, and that they are only ever modified together (see update in my example), then it is impossible for them to be seen in an inconsistent state.

e.g.

class Shared {
  private String a;
  private String b;
  public synchronized String getA() {
    return a;
  }
  public synchronized String getB() {
    return b;
  }
  public synchronized void update(String a, String b) {
    this.a = a;
    this.b = b;
  } 
}