Skip to main content
Tweeted twitter.com/#!/StackCodeReview/status/261845142317711361
Fixing a bug, based on one of the comments
Source Link
vikky.rk
  • 133
  • 1
  • 5

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();
    }
}

isLocked = true;
owner = }Thread.currentThread().getId();   
     }
}    
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();
}
}

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();
}
}

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(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();
    }
}

isLocked = true;
owner = Thread.currentThread().getId();           
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();
}
}
Source Link
vikky.rk
  • 133
  • 1
  • 5

Java Non Reentrant Lock Implementation

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();
}
}