6

Probably a very silly question. Just wanted to confirm my understanding.

class Test
{
       private volatile String id;

       public void setID(String id)
       {
             this.id = id;
       }

       public String getID()
       {
             return id;
       }
}

Lets say that an object of above class can be accessed by multiple threads. My understanding is that in case of simple getter and setters like above (doing simple initialization), I do not need to make these methods synchronized, correct ?
I guess volatile is needed as otherwise value of id can be outdated otherwise in different threads.
So, can anyone see any problem if we do not have these methods as synchronized ?

snow_leopard
  • 1,466
  • 2
  • 20
  • 36
  • Maybe this question helps you: http://stackoverflow.com/questions/15828067/java-synchronized-keyword-needed-on-primitive-getter-setter-method – pad Dec 11 '13 at 10:21
  • No you have to synchronize it in your method also – Java_Alert Dec 11 '13 at 10:22
  • this needs synchronization for volatile keyword, without it, if the POJO is not a singleton or shared via all Threads, it would not need synchro. – RamonBoza Dec 11 '13 at 10:22

4 Answers4

8

My understanding is that in case of simple getter and setters like above (doing simple initialization), I do not need to make these methods synchronized, correct ?

Correct, because what they get and set (an object reference) is treated atomically by the JVM.

The answer would be "No, you do need synchronization" if you were using a long or double and you didn't have it marked volatile.

Both aspects of this are covered in 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.

Community
  • 1
  • 1
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • just ``long``s? or does this apply to other primitives as well? – Chris Knight Dec 11 '13 at 10:22
  • Thanks, so in this case I do not even need volatile, correct ? – snow_leopard Dec 11 '13 at 10:27
  • @snow_leopard: Not for an object reference, no; you would for `long` or `double`. (Please see the updated answer -- took forever to fix, stupid broadband went down. I realized that you'd put `volatile` but I hadn't seen it. `volatile` addresses the `long` and `double` thing. The updated answer should be clearer.) – T.J. Crowder Dec 11 '13 at 10:30
  • @ChrisKnight: `long` and `double` – T.J. Crowder Dec 11 '13 at 10:31
1

If you are in a multi-threaded environment. several thread can access your data. Reading value (get) is fine.But think about the write(set), then your data will become inconsistence. So you have to Synchronized.

Ruchira Gayan Ranaweera
  • 34,993
  • 17
  • 75
  • 115
  • Yep, I forgot to add that in my case only one thread will actually set the ID And multiple threads can read that value. – snow_leopard Dec 11 '13 at 10:26
  • @Ruchira: `synchronized` makes no difference here at all, the value is set atomically by the JVM without it. – T.J. Crowder Dec 11 '13 at 10:31
  • @T.J.Crowder If we have a `synchronized` setter, only one thread will be able to set the value at a time while other will wait – Ruchira Gayan Ranaweera Dec 11 '13 at 10:33
  • @Ruchira: Right. Which is exactly what will happen *without* a synchronized setter, because the JVM will set the value atomically. There is zero difference. – T.J. Crowder Dec 11 '13 at 10:39
1

You do not need to make any of those functions synchronized nor use the volatile keyword, setting references is always atomic. There are however other problems that arise from non-synchronization/non-volatiling.

First: what Thread A reads via getID might not be what Thread B wrote via setID, because either Thread A was too early or...

Second: Thread A was on time but because of the lack of volatile, it was reading a cached thread variable instead of the real value.

While the first one can only be solved via external thread synchronization or by the architecture of your code, the second one can cause issues based on the happens-before problematic. Take the following example:

Thread A:

myId.setId(3);
idSet = true;

Thread B:

if (idSet) {
  accessData(myId.getId());
}

This looks correct - and it kind of is - but what can happen during the JVM optimization step is that first idSet = true is executed and then myId.setId(3). So in worst case, Thread B succeeds on the if clause, but then reads a wrong value. Marking id as volatile will solve that problem, as it guarantees that whenever id is modified, everything that happened before has actually happened.

Another way to solve that is to use immutable classes, so no setter and id is final and set via constructor.

TwoThe
  • 13,879
  • 6
  • 30
  • 54
0
My understanding is that in case of simple getter and setters like above
(doing simple initialization), I do not need to make these methods
 synchronized, correct ? 

Making a variable volatile simply ensures there will not be a race condition. All threads will read the same value of variable because the variable being volatile is stored directly in main memory and not in Threads local cache.

However it is possible one thread enters public String getID() function. At this point other thread may very well change the value of the variable by executing public void setID(String id) method. 1st thread will see this modified value.

So do not get confused between using atomic variables and synchronizing functions.

Aniket Thakur
  • 66,731
  • 38
  • 279
  • 289
  • While it's true that without synchronization Thread A might enter `getID`, then Thread B might change the value before Thread A reads it, making Thread A return the new value rather than the old, it makes no operational difference at all, because all `getID` does is read and return the value. It's a race either way. The only reason for synchronizing *these particular* getters and setters is data integrity (ensuring that values are read/written atomically, so we don't get half of the old value and half of the new), and that's not needed in this case. – T.J. Crowder Dec 11 '13 at 11:26