Search code examples
javamultithreadingperformanceparallel-processingsynchronized

Why race condition is only solved with ReentrantLock and not synchronized


I try to use multi-threads to add all elements to my result list. I am not sure why the result looks weird.

public class PrintMessage {

    public static void main(String[] args) {
        PrintMessage p = new PrintMessage();
        List<String> list = List.of("a", "b", "c", "d", "e", "f", "g", "h");

        ExecutorService executor = Executors.newFixedThreadPool(5);

        p.printValue(list, executor);
        try {
            executor.awaitTermination(1000, TimeUnit.MILLISECONDS);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        executor.shutdown();

        System.out.println(res);
    }

    //ReentrantLock lock = new ReentrantLock();
    private volatile int index = 0;
    static List<String> res = new ArrayList<>();

    class Printer implements Runnable {
        List<String> list;

        public Printer(List<String> list) {
            this.list = list;
        }

        @Override
        public void run() {
            //lock.lock();
           
            pickItem(index, list);
            //increment();
            //lock.unlock();
        }
    }

    private void increment() {
        index++;
    }
    private synchronized void pickItem(int index, List<String> list) {
        res.add(list.get(index));
        increment();
    }
    public void printValue(List<String> list, ExecutorService executor) {

        for (int i = 0; i < list.size(); i++) {
            executor.submit(new Printer(list));
        }
    }
}

Not sure why does the index not change by other threads, when I change synchronized and use ReentrantLock to lock pick(), increment() methods. the result looks good. Can someone explain the reason? Many thanks.

[a, b, c, e, e, f, a, a]


Solution

  • Not sure why does the index not change by other threads, when I change synchronized and use ReentrantLock to lock pick(), increment() methods. the result looks good. Can someone explain the reason? Many thanks.

    The synchronized on the method

    private synchronized void pickItem(int index, List<String> list) {
        res.add(list.get(index));
        increment();
    }
    

    is locking using the implicit lock of the object instance returned by this of the class Printer, and since for each Thread a new object instance of a the Printer class is created:

    for (int i = 0; i < list.size(); i++) {
        executor.submit(new Printer(list));
    }
    

    each thread is calling synchronized on different instances of the class Printer. Hence, those threads are not locking using the same lock, and consequently there is a race-condition.

    However, the lock variable

    ReentrantLock lock = new ReentrantLock();
    

    is a field variable of the Object PrintMessage, which is shared among all threads. Therefore, when threads invoke lock.lock(); and lock.unlock(); they are using the same (shared) lock, and consequently, there is no longer the aforementioned race-condition.

    The ReentrantLock alone (without the synchronized on the method pickItem) solves the aforementioned race-condition.