paris_serviola
paris_serviola

Reputation: 439

Are the methods thread safe?

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

Answers (1)

Mike Laren
Mike Laren

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

Related Questions