Try to implement a mutex like in java, debugging succeed but run failed-CodePudding

I want to implement a mutex in java similar to mutex in C . So i write a simple class "Mutex". Here is my code:

public class Mutex implements Lock {
    private boolean locked = false;

    //ID of the thread that owns the lock
    long ownerID;

    public Mutex() {

    }

    @Override
    public boolean tryLock() {
        if (locked) {
            //System.out.println(Thread.currentThread().getName()   " lock failed!");
            return false;
        }
        synchronized (this) {
            if (locked) {
                return false;
            }
            //System.out.println(Thread.currentThread().getName()   " lock succeed!");
            ownerID = Thread.currentThread().getId();
            return locked = true;
        }
    }

    @Override
    public boolean isLocked() {
        return locked;
    }

    @Override
    public void unLock() {
        if (!locked) {
            throw new LockException("Try to unlock an unlocked lock!");
        }
        if (Thread.currentThread().getId() != ownerID) {
            throw new LockException("Thread(ID: "   Thread.currentThread().getId()
                      ")"   " doesn't match the lock!");
        }
        //System.out.println(Thread.currentThread().getName()   " unlock!");
        locked = false;
    }
}

The results of running and debugging are different. The first thread to grab the lock seems to be able to grab the lock forever. And another thread even cann't run normanlly after the first thread end. The second thread seems to be blocked somwhere? I don't know. Help me please, thank you.

CodePudding user response：

I guess you are trying to educate yourself because it doesn't make sense to implement a Mutex based on synchronization since that already uses a mutex under the hood.

With that out of the way: one of your problems is that locked isn't volatile and hence there is no happens-before edge between writing and reading the lock. As consequence, you have a data race and this allows for weird things to happen.

I would get rid of the locked field and store the reference of the current Thread in the owner field and make that field volatile.

CodePudding user response：

There are a few problems. You can't get atomicity between your locked field and your ownerID fields without using another lock (the intrinsic locking provided by synchronized). So, use a single field that indicates who holds the lock and, by implication, whether the lock is held or not.

Next, you need a memory barrier to establish a synchronization order between writing your field and reading it in other threads. This can be achieved by declaring your field volatile.

Finally, long is not guaranteed to be read or written atomically. This can result in "tearing", where the upper and lower halves of the value come from different logical writes. (This is more of a theoretical problem on today's 64-bit architectures, but I assume you are re-inventing the wheel because you want to learn the theory—which is commendable!) Use another type, like a reference, which is guaranteed to be written and read atomically.

These recommendations lead to using a single volatile field of Thread type, which is null when the lock is released:

private volatile Thread owner = null;

Even then, you lack the atomicity or compare-and-swap behavior you need to check and set the owner of the lock. So, the intrinsic synchronized block is still required. But you only have to acquire that intrinsic lock when someone acquires your lock; not when simply checking whether it's locked.