not getting the desired result in ExecutorService

Question

Why I am not getting the list sizes of list1 and list2 as 1000 in the code below:

https://pastebin.com/ttcbkjqR

My main method code is as below:

    ExecutorService executor = Executors.newFixedThreadPool(2);

    for(int j=0; j<2; j++) {
        executor.submit(new Worker(j));
    }

    executor.shutdown();
    System.out.println("All tasks submitted: ");

    try {
        executor.awaitTermination(1, TimeUnit.DAYS);
    } catch (InterruptedException e) {

    }
    long end = System.currentTimeMillis();
    System.out.println("Time taken: " + (end - start));
    System.out.println("List1: " + list1.size() + "; List2: " + list2.size());

And here the Worker class:

class Worker implements Runnable{
    private int id;
    
    public Worker(int id) {
        this.id = id;
    }

    private Random random = new Random();
    private final Object lock1 = new Object();
    private final Object lock2 = new Object();

    public void stageOne() {
        synchronized(lock1) {
            try {
                Thread.sleep(1);
            } catch (Exception e) {
                e.printStackTrace();
            }
            WorkerThreadPool.list1.add(random.nextInt(100));
        }
    }

    public void stageTwo() {
        synchronized(lock2) {
            try {
                Thread.sleep(1);
            } catch (Exception e) {
                e.printStackTrace();
            }
            WorkerThreadPool.list2.add(random.nextInt(100));
        }
    }

    public void process() {
        for(int i=0; i<500; i++) {
            stageOne();
            stageTwo();
        }
    }

    @Override
    public void run() {
        System.out.println("Starting thread: " + id);
        process();     
        System.out.println("Completed: " + id);   
    }
}

on one time running I am getting

List1: 986; List2: 989

Other time I am getting

List1: 994; List2: 981

But each time It should give

List1: 1000; List2: 1000

What's wrong here?

Answer 1

 class Worker implements Runnable{
    
        private int id;
        
        public Worker(int id) {
            this.id = id;
        }
    
        private Random random = new Random();

        private final Object lock1 = new Object();
        private final Object lock2 = new Object();

       .....

The result is unpredictable because threads are not sharing lock.

Here, lock1 and lock2 serve no purpose.

This reason is: When you submit tasks in WorkerThreadPool using executor.submit(new Worker(j));, each thread gets its own copy of locks (each Worker instance creates new lock1 and lock2).

Here's updated Worker with sharedLock logic:

class Worker implements Runnable {

    private int id;
    Object sharedLock;

    public Worker(int id, Object sharedLock) {
        this.id = id;
        this.sharedLock = sharedLock;
    }

    private Random random = new Random();

    public void stageOne() {

        try {
            Thread.sleep(1);
        } catch (Exception e) {
            e.printStackTrace();
        }

        synchronized (sharedLock) {
            WorkerThreadPool.list1.add(random.nextInt(100));
        }
    }

    public void stageTwo() {

        try {
            Thread.sleep(1);
        } catch (Exception e) {
            e.printStackTrace();
        }

        synchronized (sharedLock) {
            WorkerThreadPool.list2.add(random.nextInt(100));
        }
    }

    public void process() {
        for (int i = 0; i < 500; i++) {
            stageOne();
            stageTwo();
        }
    }

    @Override
    public void run() {
        System.out.println("Starting thread: " + id);
        process();
        System.out.println("Completed: " + id);
    }

}

Updated WorkerThreadPool:

public class WorkerThreadPool {

    public static List<Integer> list1 = new ArrayList<>();
    public static List<Integer> list2 = new ArrayList<>();

    static Object sharedLock = new Object(); // share lock

    public static void main(String[] args) {

        long start = System.currentTimeMillis();

        ExecutorService executor = Executors.newFixedThreadPool(2);

        for (int j = 0; j < 2; j++) {
            executor.submit(new Worker(j, sharedLock)); // pass same lock to each instance of Worker
        }

        executor.shutdown();
        System.out.println("All tasks submitted: ");

        try {
            executor.awaitTermination(1, TimeUnit.DAYS);
        } catch (InterruptedException e) {

        }
        long end = System.currentTimeMillis();

        System.out.println("Time taken: " + (end - start));
        System.out.println("List1: " + list1.size() + "; List2: " + list2.size());
    }

}

Output:

All tasks submitted: 
Starting thread: 1
Starting thread: 0
Completed: 0
Completed: 1
Time taken: 1842
List1: 1000; List2: 1000