Should getters and setters be synchronized?
Asked Answered
W

4

75
private double value;

public synchronized void setValue(double value) {
    this.value = value;
}
public double getValue() {
    return this.value;
}

In the above example is there any point in making the getter synchronized?

Weltpolitik answered 12/7, 2012 at 19:50 Comment(3)
making the double field volatile would be satisfying if you are using Java 1.5 or bigger and use only set and get (setValue does not have to be synchronized then). See java.util.concurrent.atomic.Atomic* classes and the already cited Java Concurrency in Practice.Umbles
@Umbles This has worked on all Java versions.Jeb
If you are looking for synchronized only for set and get methods of protected data, have a look at other alternatives in : #9750246Ambidextrous
C
94

I think its best to cite Java Concurrency in Practice here:

It is a common mistake to assume that synchronization needs to be used only when writing to shared variables; this is simply not true.

For each mutable state variable that may be accessed by more than one thread, all accesses to that variable must be performed with the same lock held. In this case, we say that the variable is guarded by that lock.

In the absence of synchronization, the compiler, processor, and runtime can do some downright weird things to the order in which operations appear to execute. Attempts to reason about the order in which memory actions "must" happen in insufflciently synchronized multithreaded programs will almost certainly be incorrect.

Normally, you don't have to be so careful with primitives, so if this would be an int or a boolean it might be that:

When a thread reads a variable without synchronization, it may see a stale value, but at least it sees a value that was actually placed there by some thread rather than some random value.

This, however, is not true for 64-bit operations, for instance on long or double if they are not declared volatile:

The Java Memory Model requires fetch and store operations to be atomic, but for nonvolatile long and double variables, the JVM is permitted to treat a 64-bit read or write as two separate 32-bit operations. If the reads and writes occur in different threads, it is therefore possible to read a nonvolatile long and get back the high 32 bits of one value and the low 32 bits of another.

Thus, even if you don't care about stale values, it is not safe to use shared mutable long and double variables in multithreaded programs unless they are declared volatile or guarded by a lock.

Cosine answered 12/7, 2012 at 19:55 Comment(21)
@FabianBarney Absolutely, I was just about to add itCosine
To elaborate, if one wants to obtain the most recent version of value in a multi-threaded application, both the getter and setter should be synchronized. If, for some really really weird reason, you don't care about getting the latest version, and synchronization is eating up too much time (this scenario is unlikely!) I think you an get away with not synchronizing the getter. Except in this case - a double is 64 bits so it should always be synchronized.Volcanology
Don't agree on the third paragraph. AFAIK synchronized doesn't put any constraints on the order in which operations take place.Dickenson
@Volcanology Not synchronizing the getter is simply incorrect. You may never, ever observe a change to the var.Jeb
@Volcanology Not only can you not get the latest version, but you don't even know which version you will get - and that version might be different from one thread to another. That can hardly be an acceptable scenario...Aureaaureate
@Aureaaureate I did say the scenario is extremely unlikely, and only applies if, for some strange reason, you son't care about which version you get. I edit comment to say "really weird"!Volcanology
"In the absence of synchronization, the compiler, processor, and runtime can do some downright weird things to the order in which operations appear to execute" - see example 17.4.5-1 (Happens-before Consistency) in docs.oracle.com/javase/specs/jls/se7/html/…Imamate
About long and double, I think it's not true on 64-bit JVM, where there is no need to do 2 separate operation on 32-bit values, as it can do it in one single operation, due to size of computation unit, which is 64 bit).Dickenson
@gasan I think you are right, I will add it to the answer unless someone tells otherwise.Cosine
@gasan Whether the JVM is 32-bit or 64-bit is irrelevant because we're discussing this at the specification level. The JLS specifies long and double operations as non-atomic. But, all that is quite irrelevant with the new Memory Model definition where it is clear that there is no deal without proper happens-before ordering.Jeb
@MartinWilson in the spec there is no words about synchronized making one operation preceding the other, i.e. guarantee of happens-before, it only says that An unlock on a monitor happens-before every subsequent lock on that monitor, which is obvious. And doesn't preclude possible reordering of read and write.Dickenson
@gasan Entering a synchronized block means that a monitor lock is taken and leaving the block means the monitor lock is released.Cosine
@platzhirsch yes, but that doesn't mean that operations that caused that aren't reordered.Dickenson
@MarkoTopolnik you're right here. I always hoped that long and double operations are atomic on 64-bit JVM, but according to spec, it's not guaranteed.Dickenson
@platzhirsch according to what Marko Topolnik says in his answer, you're possibly right.Dickenson
@platzhirsch it seems that you're also right because of (from JLS): If an action x synchronizes-with a following action y, then we also have hb(x, y).Dickenson
I'm not clear what will happen if we don't synchronize reads and writes to a volatile long or double (when multiple threads are accessing them)? I think the re-ordering can happen but you've quoted the specification for the case: "This, however, is not true for 64-bit operations, for instance on long or double if they are not declared volatile:".Engstrom
Volatile read and synchronized write is possible. Please read securecoding.cert.org/confluence/display/java/… (see section "Compliant Solution (Volatile-Read, Synchronized-Write)"). I've used it in blog.javaslang.com/synchronizedthis.Lucy
So... should we just synchronize every getter ever?Karmakarmadharaya
@TylerPfaff If it accesses a shared field.Cosine
Marko and Assylias are right, Downvoting. Please fix and I'll reverse my vote.Antic
J
21

Let me show you by example what is a legal way for a JIT to compile your code. You write:

while (myBean.getValue() > 1.0) {
  // perform some action
  Thread.sleep(1);
}

JIT compiles:

if (myBean.getValue() > 1.0) 
  while (true) {
    // perform some action
    Thread.sleep(1);
  }

In just slightly different scenarios even the Java compiler could prouduce similar bytecode (it would only have to eliminate the possibility of dynamic dispatch to a different getValue). This is a textbook example of hoisting.

Why is this legal? The compiler has the right to assume that the result of myBean.getValue() can never change while executing above code. Without synchronized it is allowed to ignore any actions by other threads.

Jeb answered 12/7, 2012 at 20:12 Comment(14)
Is there any official source saying that w/o synchronized such permutations are possible and that synchronized precludes them to happen?Dickenson
@gasan Sure, it's the JLS. It won't point out any particular transformation, but this follows from the semantics of the Java Memory Model.Jeb
That's possibly true, but this permutation looks a bit frightening and I think it would be explicitly mentioned in the documentation.Dickenson
@gasan Everyday you are executing code where things exactly like this (some even more frightening) happen all the time.Jeb
at least I hope such an "optimisation" won't happen if inner loop changes value a check is made on.Dickenson
@gasan Well certainly, that's what theorem provers are for.Jeb
@b1naryatr0phy All you need is the JLS, specifically the Memory Model section. All allowed optimizations are the consequence of that model.Jeb
@MarkoTopolnik Thanks Marko I'm seeing it now, I appreciate it.Bandeen
How can this optimization be legal? It is illegal even in a single-thread scenario. The "some action" inside the while-loop in the compiled version could change the value of myBean. However, after passing the if-condition, the value is not even checked, even though it may have changed inside the while-loop. And even if the value is not changed inside the while-loop, and could be changed by a different thread - how would synchronizing on getValue() avoid getting into an infinite loop?Oxyacetylene
@AjoyBhatia "some action" either will or won't change myBean and the JIT will know either way. If it determines it might change, then it won't do the optimization. As for other threads, that's basic Java Memory Model semantics: without synchronization/volatile, the runtime can ignore the actions of other threads when reasoning about the code.Jeb
if my getter is public double getValue() { synchronized(this){return value} } will the jit not see the synchronized and do this same thing?Beefwitted
@Beefwitted You never have to worry about the JIT making an illegal code transformation.Jeb
@MarkoTopolnik Okay. My question would be then whether it would be a legal code transformation for the JIT to do the above? And if not, why is that illegal, but legal for the OPs code above. It's the same method signature.Beefwitted
@Beefwitted Your method's implementation involves locking; OP's doesn't. As simple as that.Jeb
M
2

The reason here is to guard against any other thread updating the value when a thread is reading and thus avoid performing any action on stale value.

Here get method will acquire intrinsic lock on "this" and thus any other thread which might attempt to set/update using setter method will have to wait to acquire lock on "this" to enter the setter method which is already acquired by thread performing get.

This is why its recommended to follow the practice of using same lock when performing any operation on a mutable state.

Making the field volatile will work here as there are no compound statements.


It is important to note that synchronized methods use intrinsic lock which is "this". So get and set both being synchronized means any thread entering the method will have to acquire lock on this.


When performing non atomic 64 bit operations special consideration should be taken. Excerpts from Java Concurrency In Practice could be of help here to understand the situation -

"The Java Memory Model requires fetch and store operations to be atomic, but for non-volatile long and double variables, the JVM is permitted to treat a 64 bit read or write as two separate 32 bit operations. If the reads and writes occur in different threads, it is therefore possible to read a non-volatile long and get back the high 32 bits of one value and the low 32 bits of another. Thus, even if you don't care about stale values, it is not safe to use shared mutable long and double variables in multi-threaded programs unless they are declared volatile or guarded by a lock."

Melodeemelodeon answered 22/4, 2014 at 7:37 Comment(0)
M
-3

Maybe for someone this code looks awful, but it works very well.

  private Double value;
  public  void setValue(Double value){
    updateValue(value, true);
  }
  public Double getValue(){
      return updateValue(value, false);
  }
  private double updateValue(Double value,boolean set){
    synchronized(MyClass.class){
      if(set)
        this.value = value;
      return value;
    }
  }
Maria answered 23/5, 2017 at 9:5 Comment(6)
It does look awful, yes. I don't see how this is better than the standard practice of making the getter and setter simple synchronized one-liner methods: this is 2x more code, it is far less readable because it looks highly unusual (so it distracts a reader familiar with common Java practices). It also synchronizes on Double.class, which effectively makes this a global lock rather than per-instance lock. What's the point really?Auberon
Check the subject please. Class synchronization does not block the class, only blocks the synchronization block. Sometimes our getters and setters are more complicated. In the body methods we do different things and we want it to be safe in many threads. The update function will pause another thread, for example if we take a read. Check and see for yourself that you will not do this by simply synchronizing a variable.Maria
The synchronized block blocks on the value that you pass to it, you're passing Double.class which is a global object in the JVM.Auberon
In my case it was a static method. The class was different, double was here for example. Maybe not the best example, another class will be viscose. What's not to blame is the fact that blocking is only for synchronized block.Maria
Okay. I mean that this code will block against any other code in the same JVM that also says synchronized (Double.class).Auberon
@Auberon - You were correct the first time. It is what we would normally refer to as a global lock. This is not the same as a global interpreter lock as you find in some PLs. But it is more likely to be a bottleneck than an instance lock. So this solution is definitely inferior from a performance perspective ... as well as being "odd".Martinamartindale

© 2022 - 2024 — McMap. All rights reserved.