2

I have implemented a Non Reentrant Lock. I want to know if this has any mistakes, race conditions etc. I am aware of the fact that existing libraries have to be used (instead of writing our own), but this is just to see if I am understanding the java concurrency correctly. Any feedback is appreciated.

public class MyLock {
private boolean isLocked = false;
private long owner = -1;
private static String TAG = "MyLock: ";

public synchronized void Lock() throws InterruptedException, IllegalStateException {
    if(!isLocked) {
        isLocked = true;
        owner = Thread.currentThread().getId();

    } else {
        if(owner == Thread.currentThread().getId()) {
            throw new IllegalStateException("Lock already acquired. " +
                                            "This lock is not reentrant");
        } else {
            while(isLocked == true) {
                System.out.println(TAG+"Waiting for Lock, Tid = " +
                        Thread.currentThread().getId());
                wait();
            }   
        }
    }
    System.out.println(TAG+"Lock Acquired: Owner = " + owner);
}

public synchronized void Unlock() throws IllegalStateException {
    if(!isLocked || owner != Thread.currentThread().getId()) {
        throw new IllegalStateException("Only Owner can Unlock the lock");
    } else {
        System.out.println(TAG+"Unlocking: Owner = " + owner);
        owner = -1;
        isLocked = false;
        notify();
    }
}

}

vikky.rk
  • 3,989
  • 5
  • 29
  • 32
  • 3
    Move to http://codereview.stackexchange.com/ – Darryl Miles Oct 25 '12 at 09:56
  • The decision to have Locks as `Reetrant` was taken after lot of thought. It is not advisable to implement `Non-Reetrant` lock just for the sake of doing it? What is the thought process you have put behind it? Why do you require such thing? – Amit Deshpande Oct 25 '12 at 09:56
  • @DarrylMiles: Is there a way to move it to codereview? Or should I do create another post in codereview. – vikky.rk Oct 25 '12 at 10:01
  • @AmitD: http://stackoverflow.com/questions/187761/recursive-lock-mutex-vs-non-recursive-lock-mutex has a good discussion on rentrant vs non-reentrant locks. – vikky.rk Oct 25 '12 at 10:02
  • here is a non-reentrant lock: `new java.util.concurrent.Semaphore(1)` – bestsss Oct 25 '12 at 10:28
  • 1
    Here is a simple review: `Thread.currentThread().getId()` is useless for anything but monitoring/logging (or JMX to query info, which in affect is useful for monitoring/logging only). Remove getId() and use the thread itself. Hint: `getId()` can be overridden. – bestsss Oct 25 '12 at 10:30

1 Answers1

0

Here is an implementation of a "standard" / "non-reentrant" lock in Java, as a wrapper around Java's built-in ReentrantLock that simply prevents the lock from ever being acquired more than once.

/**
 * A "non-reentrant" lock, implemented as a wrapper around Java's ReentrantLock.
 *
 */
class StandardLock implements java.util.concurrent.locks.Lock {

    public static class LockAlreadyHeldException extends RuntimeException {}

    private final java.util.concurrent.locks.ReentrantLock mainLock;

    private void checkNotAlreadyHeld() {
        if (mainLock.getHoldCount()!=0) {
            throw new LockAlreadyHeldException();
        }
    }

    public StandardLock() {
        mainLock=new java.util.concurrent.locks.ReentrantLock();
    }

    public StandardLock(boolean fair) {
        mainLock=new java.util.concurrent.locks.ReentrantLock(fair);
    }

    @Override
    public void lock() {
        checkNotAlreadyHeld();
        mainLock.lock();
    }

    @Override
    public void lockInterruptibly() throws InterruptedException {
        checkNotAlreadyHeld();
        mainLock.lockInterruptibly();
    }

    @Override
    public boolean tryLock() {
        checkNotAlreadyHeld();
        return mainLock.tryLock();
    }

    @Override
    public boolean tryLock(long time, TimeUnit unit) throws InterruptedException {
        checkNotAlreadyHeld();
        return mainLock.tryLock(time, unit);
    }

    @Override
    public void unlock() {
        mainLock.unlock();
    }

    @Override
    public Condition newCondition() {
        return mainLock.newCondition();
    }
}

The advantages to this approach are that the class implements Java's Lock interface, and Condition Variables thus come with it in order to allow the creation of Monitors. Monitors are important in order to fully leverage locks for concurrent programming.

user553702
  • 2,819
  • 5
  • 23
  • 27