Java:Multiple thread safety issues: Using Thread extends and Lock

Question

When I use Thread class and Lock to solve threads safety problems, there occurs 0 ticket!

Code:

package ThreadTest;

import java.util.concurrent.locks.ReentrantLock;

class ThreadOne extends Thread{
    private static int ticket = 100;
    private ReentrantLock lock = new ReentrantLock();
    @Override
    public void run() {
        while(true){
            try {
                lock.lock();
                if(ticket > 0){
                    try {
                        Thread.sleep(100);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    System.out.println(Thread.currentThread().getName()+"selling ticket: "+ticket);
                    ticket--;
                }else{
                    break;
                }
            } finally {
                lock.unlock();
            }
        }
    }
}

public class ThreadMethod {
    public static void main(String[] args) {
        ThreadOne t1 = new ThreadOne();
        ThreadOne t2 = new ThreadOne();

        t1.setName("Window 1:");
        t2.setName("Window 2:");

        t1.start();
        t2.start();
    }
}

And the result contains: Window 2:selling ticket: 3 Window 1:selling ticket: 2 Window 2:selling ticket: 1 Window 1:selling ticket: 0

0 ticket!

so you mean that we cannot use Thread extends and Lock at the same? — CarterZhang, Jan 27 '21 at 02:47
You're using separate locks on each thread. I think the threads have to share the same lock objects to do this. — NomadMaker, Jan 27 '21 at 02:49
I know that this isn't associated with your question, but I would urge you to consider implementing the Runnable interface instead of extending Thread: https://stackoverflow.com/q/541487/42962 — hooknc, Jan 27 '21 at 02:49
Right, you declare `lock` as an instance variable. This means each `ThreadOne` has it's own copy of the lock, and as a result it actually locks nothing. Try declaring the lock to be `static`. — markspace, Jan 27 '21 at 02:55

score 2 · Answer 1 · answered Jan 27 '21 at 03:00

The first problem is that you have two independent locks per each instance of ThreadOne, so you are not actually locking anything. There is no competition to even try to acquire the lock between the threads, because they both have their own. That's easy to fix if you make that ReentrantLock static.

If you do make it static though, you will notice that only one of the "windows" will keep getting tickets. That has to do with the way ReentrantLock works internally and the fact that you are using a while(true){...}. That is easy to fix too. Either:

change the lock to be fair: private static ReentrantLock LOCK = new ReentrantLock(true);
or sleep a bit after the finally block, to give the other thread the chance to run.

Like this:

.....

} finally {
   LOCK.unlock();
}
        
try {
    Thread.sleep(10);
}
catch (InterruptedException e) {
    e.printStackTrace();
}

Java:Multiple thread safety issues: Using Thread extends and Lock

1 Answers1