Correctly synchronizing equals() in Java

Question

I have the following class which contains only one field i. Access to this field is guarded by the lock of the object ("this"). When implementing equals() I need to lock this instance (a) and the other (b). If thread 1 calls a.equals(b) and at the same time thread 2 calls b.equals(a), the locking order is reverse in the two implementations and may result in deadlock.

How should I implement equals() for a class which has synchronized fields?

public class Sync {
    // @GuardedBy("this")
    private int i = 0;
    public synchronized int getI() {return i;}
    public synchronized void setI(int i) {this.i = i;}

    public int hashCode() {
        final int prime = 31;
        int result = 1;
        synchronized (this) {
            result = prime * result + i;
        }
        return result;
    }

    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        Sync other = (Sync) obj;
        synchronized (this) {
            synchronized (other) {
                // May deadlock if "other" calls 
                // equals() on "this" at the same 
                // time 
                if (i != other.i)
                    return false;
            }
        }
        return true;
    }
}

Answer 1

Trying to synchronize equals and hashCode inside the object will not work properly. Consider the case of a HashMap that uses hashCode to discover which "bucket" an object will be in, and then uses equals to sequentially search all objects in the bucket.

If objects are allowed to mutate in a way that changes the outcomes of hashCode or equals you could end up with a scenario where HashMap calls hashCode. It acquires the lock, gets the hash and releases the lock again. HashMap then proceeds to compute which "bucket" to use. But before HashMap can acquire the lock on equals someone else grabs the lock and mutates the object so that equals become inconsistent with the previous value of hashCode. This will lead to catastrophic results.

The hashCode and equals methods are used in a lot of places and is core to the Java collections API. I might be valuable to rethink your application structure that do not require synchronized access to these methods. Or at the very least not synchronize on the object itself.