Reputation: 439
This HardwareData class is simulates the idea of get-and-set and swap instructions in concurrent programming. I used synchronized statement to make sure while doing getAndSet or Swap no other get/set/swap/getandset is happening? Do you guys think this is thread sage?
public class HardwareData {
private boolean value = false;
public HardwareData(boolean value){
this.value = value;
}
public synchronized boolean get(){
return value;
}
public synchronized void set(boolean newValue){
value = newValue;
}
public boolean getAndSet(boolean newValue){
synchronized(this) {
boolean oldValue = this.get();
this.set(newValue);
return oldValue;
}
}
public void swap(HardwareData other){
synchronized(this){
boolean temp = this.get();
this.set(other.get());
other.set(temp);
}
}
Upvotes: 0
Views: 296
Reputation: 8188
Definitely not. The swap
method is not thread safe because you don't hold the mutex of the other
object during:
this.set(other.get());
other.set(temp);
meaning that a third thread might be able to call other.set(boolean)
while you're in between these two lines and therefore change the internal state of other
. There's also the risk of ending up in a deadlock if two calls to object1.swap(object2)
and object2.swap(object1)
happen at the same time.
To solve this problem, the swap
method must be synchronizing on the two objects at the same time, which is tricky because it can lead to deadlocks depending on the order they're acquired.
Also: adding synchronize
to the method declaration is the same as wrapping the entire method body in synchronized(this){ ... }
.
Upvotes: 1