CODEWITHSUNDEEP
javamultithreadingthread-safetyatomic
Gurwinder Singh
Gurwinder Singh

Reputation: 39537

Volatile keyword for thread safety

I have the below code:

public class Foo {
  private volatile Map<String, String> map;

  public Foo() {
    refresh();
  }

  public void refresh() {
    map = getData();
  }

  public boolean isPresent(String id) {
    return map.containsKey(id);
  }

  public String getName(String id) {
    return map.get(id);
  }

  private Map<String, String> getData() {
    // logic
  }

}

Also, I've read that one should use AtomicReference instead of this, but in the source of the AtomicReference class, I can see that the field used to hold the value is volatile (along with a few convenience methods).

I've read multiple answer related to this but the concept of volatile still confuses me. Thanks in advance.

Upvotes: 1

Views: 528

Answers (4)

Solomon Slow
Solomon Slow

Reputation: 27210

A class is "thread safe" if it does the right thing when it is used by multiple threads at the same time. There is no way to tell whether a class is thread safe unless you can say what "the right thing" means, and especially, what "the right thing when used by multiple threads" means.

What is the right thing if thread A calls foo.isPresent("X") and it returns true, and then thread B calls foo.refresh(), and then thread A calls foo.getName("X")?

If you are going to claim "thread safety", then you must be very explicit about what the caller should expect in cases like that.

Upvotes: 2

Murat Karag&#246;z
Murat Karag&#246;z

Reputation: 37624

If I understood your question correctly and your comments - your class Foo holds a Map in which only the reference should be updated e.g. a whole new Map added instead of mutating it. If this is the premise:

It does not make any difference if you declare it as volatile or not. Every read/write operation in Java is atomic itself. You will never see a half transaction on these operations. See the JLS 17.7

17.7. Non-Atomic Treatment of double and long

For the purposes of the Java programming language memory model, a single write to a non-volatile long or double value is treated as two separate writes: one to each 32-bit half. This can result in a situation where a thread sees the first 32 bits of a 64-bit value from one write, and the second 32 bits from another write.

Writes and reads of volatile long and double values are always atomic.

Writes to and reads of references are always atomic, regardless of whether they are implemented as 32-bit or 64-bit values.

Some implementations may find it convenient to divide a single write action on a 64-bit long or double value into two write actions on adjacent 32-bit values. For efficiency's sake, this behavior is implementation-specific; an implementation of the Java Virtual Machine is free to perform writes to long and double values atomically or in two parts.

Implementations of the Java Virtual Machine are encouraged to avoid splitting 64-bit values where possible. Programmers are encouraged to declare shared 64-bit values as volatile or synchronize their programs correctly to avoid possible complications.

EDIT: Although the top statement still stands as it is - for thread safety it's necessary to add volatile to reflect the immediate update on different Threads to reflect the reference update. The behavior of a Thread is to make local copy of it while with volatile it will do a happens-before relationship in other words the Threads will have the same state of the Map.

Upvotes: 0

Kayaman
Kayaman

Reputation: 73578

If you're not modifying the contents of map (except inside of refresh() when creating it), then there are no visibility issues in the code.

It's still possible to do isPresent(), refresh(), getName() (if no outside synchronization is used) and end up with isPresent()==true and getName()==null.

Upvotes: 2

Christian
Christian

Reputation: 22353

Volatile is only useful in this scenario to update the value immediately. It doesn't really make the code by itself thread-safe.

But because you've stated in your comment, you only update the reference and because the reference-switch is atomic, your code will be thread-safe.(from the given code)

Upvotes: 0

Related Questions