Java Synchronized Threads not working as expected

Question

The following code does not work as I expect it to:

import java.util.ArrayList;
import java.util.List;
import java.util.Random;

class Worker implements Runnable {

    public void run() {
        System.out.println("Started.");
        process();

    }
    private Random random = new Random();

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

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

    public void stageOne() {

        synchronized (lock1) {

            try {
                Thread.sleep(1);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            list1.add(random.nextInt(100));
        }

    }

    public void stageTwo() {

        synchronized (lock2) {
            try {
                Thread.sleep(1);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            list2.add(random.nextInt(100));
        }
    }

    public void process() {
        for(int i=0; i<1000; i++) {
            stageOne();
            stageTwo();
        }
    }
    public static void show() {
        System.out.println("List1: " + list1.size());
        System.out.println("List2: " + list2.size());
    }
}

public class JavaTest {

    public static void main(String[] args) {

        long start = System.currentTimeMillis();

        Thread t1 = new Thread(new Worker());
        t1.start();
        Thread t2 = new Thread(new Worker());
        t2.start();

        try {
            t1.join();
            t2.join();
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

        System.out.println("Completed.");

        long end = System.currentTimeMillis();

        System.out.println("Time taken: " + (end - start));

        Worker.show();
    }

}

When I run this program, I expect list1 and list2 to contain 2000 items each and for the program to take approximately 2000 milliseconds. However, lots of times I get lists less than 2000 items, although it does finish around 2000 milliseconds. Sometimes I even get a ArrayOutOfBounds Exception

Started.
Started.
Exception in thread "Thread-1" java.lang.ArrayIndexOutOfBoundsException: 163
    at java.util.ArrayList.add(ArrayList.java:459)
    at Worker.stageOne(JavaTest.java:34)
    at Worker.process(JavaTest.java:53)
    at Worker.run(JavaTest.java:14)
    at java.lang.Thread.run(Thread.java:748)
Completed.
Time taken: 2217
List1: 1081
List2: 1079

I expect that each the locks in stageOne and stageTwo should stop the threads from interfering with each other. But that does not seem to be the case. What is the problem with this code?

Answer 1

Your lock objects are not static, so each thread is synchronizing on a different monitor. So the locks are not having any effect at all.