Take the 2-minute tour ×

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 = " +
    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;


share|improve this question
Move to codereview.stackexchange.com –  Darryl Miles Oct 25 '12 at 9: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 9: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: stackoverflow.com/questions/187761/… has a good discussion on rentrant vs non-reentrant locks. –  vikky.rk Oct 25 '12 at 10:02
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

Your Answer



Browse other questions tagged or ask your own question.