Java assignment issues - Is this atomic?

Question

I've got some questions about Java's assigment.

• Strings

I've got a class:

public class Test {
 private String s;

 public synchronized void setS(String str){
  s = s + " - " + str;
 }

 public String getS(){
  return s;
 }
}

I'm using "synchronized" in my setter, and avoiding it in my getter, because in my app, there are a tons of data gettings, and very few settings. Settings must be synchronized to avoid inconsistency. My question is: is getting and setting a variable atomic? I mean, in a multithreaded environment, Thread1 is about to set variable s, while Thread2 is about to get "s". Is there any way the getter method could get something different than the s's old value or the s's new value (suppose we've got only two threads)? In my app it is not a problem to get the new value, and it is not a problem to get the old one. But could I get something else?

• What about HashMap's getting and putting?

considering this:

    public class Test {
        private Map<Integer, String> map = Collections.synchronizedMap(new HashMap<Integer, String>());

        public synchronized void setMapElement(Integer key, String value){
         map.put(key, value);
        }

        public String getValue(Integer key){
         return map.get(key);
        }
}

Is putting and getting atomic? How does HashMap handle putting an element into it? Does it first remove the old value and put the now one? Could I get other than the old value or the new value?

Thanks in advance!

Answer 1

In the first case, String happens to be safe for unsafe publication (in the "new" Java Memory Model (JMM)), so this is okay.

Not being volatile there is theoretically some issue with not having an up to date value, but then the meaning of up to date is not clear. You could replace the lock with a compare-sand-swap (CAS) loop, but that probably wouldn't give you much performance gain whether the lock was likely to be contended or not.

In the case of HashMap, an unsynchronized map is not safe to read if there is another thread writing to it, even a single writer thread. In fact, this has been found to lead to infinite loops on production systems running popular software. The code in the question actually uses two locks for the map, which is over the top (although you'd need an explicit hold of the same lock if using an iterator). Not being final does stop the containing class from being safe for unsafe publication. If map was volatile and you created a new map for every put, then that could be made to be safe without synchronisation on the get.

Answer 2

Earlier answers are correct in pointing that with new (1.5+) JVMs, String version is safe, with respect to data corruption. And you seem to be aware of downside non-synchronized access; that of changes not necessarily being visible via getters.

However: more useful question is this: is there reason to synchronization here? If this is just for being interested in knowing this, that's all good. But for real code, the general rule of reads and writes is that if there can be both, both should be synchronized. So although you can in this case leave out synchronization (potentially meaning that threads do not see changes other threads make), there seems to be little benefit in doing that.

Answer 3

Perhaps a Read Write Lock could solve your problem?

Take a look at its documentation:

A read-write lock allows for a greater level of concurrency in accessing shared data than that permitted by a mutual exclusion lock. It exploits the fact that while only a single thread at a time (a writer thread) can modify the shared data, in many cases any number of threads can concurrently read the data (hence reader threads). In theory, the increase in concurrency permitted by the use of a read-write lock will lead to performance improvements over the use of a mutual exclusion lock. ....

Answer 4

Rather than wrapping your HashMap in something to make it synchronized, consider using a java.util.concurrency.ConcurrentHashMap instead.

This is an updated version of HashMap that guarantees "Retrievals reflect the results of the most recently completed update operations holding upon their onset.".

Answer 5

In multi-threaded environment you will need to synchronize the getter in order to ensure the client sees the most up to date value of s.