Storing object reference into a volatile field

I'm using the following field:

private DateDao dateDao;

private volatile Map<String, Date> dates;

public Map<String, Date> getDates() {
    return Collections.unmodifiableMap(dates);
}

public retrieveDates() {
     dates = dateDao.retrieveDates();
}

Where

public interface DateDao {
    //Currently returns HashMap instance
    public Map<String, Date> retrieveDates();
}

Is it safe to publish the map of dates that way? I mean, volatile field means that the reference to a field won't be cached in CPU registers and be read from memory any time it is accessed.

So, we might as well read a stale value for the state of the map because HashMap doesn't do any synchronization.

Is it safe to do so?

UPD: For instance assume that the DAo method implemented in the following way:

public Map<String, Date> retrieveDates() {
    Map<String, Date> retVal = new HashMap<>();
    retVal.put("SomeString", new Date());
    //ad so forth...
    return retVal;
}

As can be seen, the Dao method doesn't do any synchronization, and both HashMap and Date are mutable and not thread safe. Now, we've created and publish them as it was shown above. Is it guaranteed that any subsequent read from the dates from some another thread will observe not only the correct reference to the Map object, but also it's "fresh" state.

I'm not sure about if the thread can't observe some stale value (e.g. dates.get("SomeString") returns null)

Solution

I think you're asking two questions:

Given that DAO code, is it possible for your code using it to use the object reference it gets here:
```
 dates = dateDao.retrieveDates();
```

before the dateDao.retrieveDates method as quoted is done adding to that object. E.g., do the memory model' statement reordering semantics allow the retrieveDates method to return the reference before the last put (etc.) is complete?

Once your code has the dates reference, is there an issue with unsynchronized access to dates in your code and also via the read-only view of it you return from getDates.

Whether your field is volatile has no bearing on either of those questions. The only thing that making your field volatile does is prevent a thread calling getDates from getting an out-of-date value for your dates field. That is:

Thread A                                       Thread B
----------                                     --------
1. Updates `dates` from dateDao.retrieveDates
2. Updates `dates` from " " again
3.                                             getDates returns read-only
                                               view of `dates` from #1

Without volatile, the scenario above is possible (but harmless). With volatile, it isn't, Thread B will see the value of dates from #2, not #1.

But that doesn't relate to either of the questions I think you're asking.

Question 1

No, your code in retrieveDates cannot see the object reference returned by dateDao.retrieveDates before dateDao.retrieveDates is done filling in that map. The memory model allows reordering statements, but:

...compilers are allowed to reorder the instructions in either thread, when this does not affect the execution of that thread in isolation

(My emphasis.) Returning the reference to your code before dateDao.retrieveDates would obviously affect the execution of the thread in isolation.

Question 2

The DAO code you've shown can never modify the map it returns to you, since it doesn't keep a copy of it, so we don't need to worry about the DAO.

In your code, you haven't shown anything that modifies the contents of dates. If your code doesn't modify the contents of dates, then there's no need for synchronization, since the map is unchanging. You might want to make that a guarantee by wrapping dates in the read-only view when you get it, rather than when you return it:

dates = Collection.unmodifiableMap(dateDao.retrieveDates());

If your code does modify dates somewhere you haven't shown, then yes, there's potential for trouble because Collections.unmodifiableMap does nothing to synchronize map operations. It just creates a read-only view.

If you wanted to ensure synchronization, you'd want to wrap dates in a Collections.synchronizedMap instance:

dates = Collections.synchronizedMap(dateDao.retrieveDates());

Then all access to it in your code will be synchronized, and all access to it via the read-only view you return will also be synchronized, as they all go through the synchronized map.