ReentrantLock lock and unlock by always the same thread

Question

I am trying to implement Reentrant locks on multi-threads but for some reason, the same thread unlocks and then locks again resulting in always the same thread to run therefore the same operation.

Below is the code how the threads are spawn

IntStream.range(0,(NUMBER_OF_THREADS)).forEach(index ->{
            boolean operation = (index % 2 == 0) ? true : false;
            Thread t = new Thread(new Client(operation,this));
            t.start();
});

and here is how the run function of the thread works

@Override
public void run() {
    while(!Thread.interrupted()) {
        System.out.println("Trying to acquire lock : " + main.getLock().tryLock()
                + " thread id " + Thread.currentThread().getName());
       // if (main.getLock().tryLock()) {
        try {
            main.getLock().lock();
            if(main.getLock().isHeldByCurrentThread()) {
                System.out.println("Lock held by this thread " + main.getLock().isHeldByCurrentThread()
                        + " thread id : " + Thread.currentThread().getName());
                if (operation) {
                    main.getcAaccount().deposit(1);
                } else {
                    main.getcAaccount().withdraw(2);
                }
                Thread.currentThread().sleep(3000);
            }
        } catch (InterruptedException e) {
                e.printStackTrace();
        } finally {
            System.out.println("Thread id : " + Thread.currentThread().getName() + " unlocking");
            main.getLock().unlock();//always have the unlock part here to ensure it unlock
        }
}

It correctly prints that the other 5 threads are trying to acquire the lock and failing and then Thread id...is unlocking...and immediately the same thread locks again even though it should be sleeping.

Have I missed anything in this logic scenario?

Thank you in advance.

EDIT Screenshot of the suggested fix.

Why do you call `lock()` when you already acquired the lock via `tryLock()`? — Robert, Mar 14 '19 at 13:14
Blocked concurrency where one thread deposits and the other wirthdraws but order doesnt matter. I just dont understand why the same thread unlocks and then acquires the lock when it should be sleeping — Eilleen, Mar 14 '19 at 13:16

John Vint · Accepted Answer · 2019-03-14T13:40:23.310

3

Reentrancy requires each lock to be followed by a subsequent unlock. For example, if I invoke lock.lock() three times, it's expected that I also invoke lock.unlock() three times. The ReentrantLock will not consider itself unlocked until this sequence of events occurs.

What you don't realize is that lock.tryLock(), if successful, will act essentially like invoking lock.lock(). So by locking twice, you need to also unlock twice. In your code example, you are unlocking only once and so the thread that initially locked still technically owns the lock.

Fixing it should be simple, you can remove the second lock.lock() from your code and the mutual exclusion should still hold. Either that, or if you need blocking on lock, then replace lock.tryLock() with lock.lock().

Based on your edit, you fixed one issue with removing the extra lock, but now you are running into a timing issue. You don't really need the tryLock. You can replace it with lock since a lock invocation will suspend the thread and block if the lock is already being held (eventually waking up when an unlock is invoked).

edited Mar 14 '19 at 13:40

answered Mar 14 '19 at 13:28

John Vint

39,695
7
78
108

I have done as you said but still the same problem. I will edit post a screenshot with your suggested fix. – Eilleen Mar 14 '19 at 13:36
Ok, then you are running into a timing issue. Consider my second point of replacing the `tryLock` with just `lock`. There is no need to `sleep` as a thread will be blocked on `lock()` anyway. – John Vint Mar 14 '19 at 13:38
I removed the if(lock.tryLock()) and now only the same thread works on its own. Weird stuff. I mean the other 5 threads don't print any attempt to acquire the lock. I will post the updated code for you – Eilleen Mar 14 '19 at 13:43
And you replaced tryLock with `lock.lock()`? – John Vint Mar 14 '19 at 13:46
I have a hunch the unlock is never invocked – Eilleen Mar 14 '19 at 13:49
The only change I'd make is to move the `lock.lock()` above the `try`, but otherwise, things look good. Are you never seeing "Thread Id: ... unlocking"? – John Vint Mar 14 '19 at 13:51
Still the same problem. So basically the first time x thread gets the lock and the rest return false. And then that thread who got the lock first just sits there unlocking and locking on its own. Recreate my code if you like. the deposit and withdraw are simple increment and decrement functions. – Eilleen Mar 14 '19 at 14:11
Ah, I missed `main.getLock().tryLock()` in the log statement. That is a locking operation (as we discussed earlier). – John Vint Mar 14 '19 at 14:18
1

Oh my god, like I always say its the simple and silly things that destroy the complex stuff. Thank you again. Works like a charm, different threads get the lock now and the cycle goes on in an arbitrary manner(whichever thread get the lock) between deposit and withdraw. Thank you again for your time. I have upvoted and accepted as answer. – Eilleen Mar 14 '19 at 14:25
Glad to help! The silliest things sometimes take the most amount of time. Good luck! – John Vint Mar 15 '19 at 01:37

ReentrantLock lock and unlock by always the same thread

1 Answers1