Mutual Exclusion using JAVA 2-Threads Solution

Question

I am trying to implement 2-threads solution using LockOne(Mutually Exclusive) Algorithm. Implementing this algorithm, i am trying work out a thread defining my own lock methods but i am not getting the desired output. When i run the program..all i get as output is "Thread-0 Locked" and "Thread-1 Locked"..Can anyone plz let me know where am i going wrong? my code is given below

public class Main {

    static MyLock lock=null;
    static int count=0;

    public static void main(String[] args) throws InterruptedException {

        Thread[] threads=new Thread[2];

        threads[0]=new Thread(new MyThread());
        threads[1]=new Thread(new MyThread());

        lock=new MyLock();

        threads[0].start();
        threads[1].start();
        threads[0].join();
        threads[1].join();

        System.out.println(count);
    }

}

public class MyLock{

    private boolean locked=false;
    private String current; 

    public void lock() {

        if(!locked){
            locked=true;
            current=Thread.currentThread().getName();
        }

        System.out.println(Thread.currentThread().getName()+" locked");
        while(locked && current!=Thread.currentThread().getName());

    }


    public void unlock() {

        System.out.println(Thread.currentThread().getName()+" unlocked");
        locked=false;

    }
}

public class MyThread implements Runnable{

    @Override
    public void run() {

        int i=1;
        while(i<=100){
            Main.lock.lock();
            Main.count++;
            Main.lock.unlock();
            i++;
        }
    }

}

200 should be the output since the count is incremented 100 times by each of the threads — Trijit, Jan 23 '18 at 01:59
On my machine, the program prints 200 and exits. Perhaps it depends on the number of available CPU cores. Either way, your lock is not thread-safe now: - there is no happens-before relationship between calls of `lock()` and `unlock()` from different threads. There is no guarantee that one thread's changes to a state of `MyLock` object will be visible to other threads. - locking process is not atomic. — Igor Melnichenko, Jan 23 '18 at 02:07
I m running this program on quad core i5 laptop. 2ndly...MyLock object here is static and public....so there is only one instance — Trijit, Jan 23 '18 at 02:24
Static and public modifiers don't make mutable object thread-safe. Check this: https://docs.oracle.com/javase/tutorial/essential/concurrency/memconsist.html — Igor Melnichenko, Jan 23 '18 at 03:25

xingbin · Answer 1 · 2018-01-23T03:04:39.483

There are two problems in your code.

if(!locked) and set locked=true is not atomic operation, which means two threads can find it not locked and lock it at same time.
There is no syncronization on varible locked and current, so one thread may not read the fresh value the other thread set on them because of memory barriar.

You can solve this with AtomicBoolean and volatile:

import java.util.concurrent.atomic.AtomicBoolean;

public class MyLock{

    private AtomicBoolean locked = new AtomicBoolean(false);
    private volatile String current;

    public void lock() {
        for (;;) {
            if(!locked.get()){
                if (locked.compareAndSet(false, true)) {
                    current = Thread.currentThread().getName();
                    System.out.println(current + " locked");
                    return;
                }
            }
        }
    }


    public void unlock() {
        System.out.println(current + " unlocked");
        locked.set(false);
    }
}

score 1 · Answer 2 · answered Jan 23 '18 at 03:42

And another solution in case if you need reentrant lock with fair acquisition policy:

public class MyLock
{
    private String holder;
    private Queue<Long> next = new ConcurrentLinkedQueue<>();

    private Object syncLock = new Object();

    public void lock()
    {
        Long threadId = Thread.currentThread().getId();
        this.next.add(threadId);
        boolean acquired = false;

        while (!acquired)
        {
            synchronized (this.syncLock)
            {
                if (this.holder == null)
                {
                    if (this.next.peek() == threadId)
                    {
                        this.holder = Thread.currentThread().getName();
                        System.out.println(this.holder + " locked");

                        acquired = true;
                    }
                }
                else
                {
                    if (this.holder.equals(Thread.currentThread().getName()))
                    {
                        acquired = true;
                    }
                }
            }
        }

        this.next.remove(threadId);
    }

    public void unlock()
    {
        synchronized (this.syncLock)
        {
            System.out.println(Thread.currentThread().getName() + " unlocked");
            this.holder = null;
        }
    }
}

Actually i know how to use reentrant locks... i just wanted to implement my version of mutual exclusion for 2 thread solution..but thanks... — Trijit, Jan 23 '18 at 18:26

Mutual Exclusion using JAVA 2-Threads Solution

2 Answers2