Volatile keyword for thread safety

Question

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
  }

}

• Is the above code thread safe or do I need to add synchronized or mutexes in there? If it's not thread safe, please clarify why.

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).

• Is there a specific reason to use AtomicReference instead?

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

Answer 1

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.