0

A quick (I think) concurrency question: I'm going through a multithreading course at Udemy.com, and the teacher talked through the code below. Although he explained it, I'm still not sure why you would create the lock1 and lock2 objects rather than locking on list1 and list2.

App.java:

public class App {

    public static void main(String[] args) {
        Worker worker = new Worker();
        worker.main();
    }
}

Worker.java:

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


public class Worker {

    private Random random = new Random();

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

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

    public void stageOne() {

        synchronized (lock1) {
            try {
                Thread.sleep(1);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }

            list1.add(random.nextInt(100));
        }

    }

    public void stageTwo() {

        synchronized (lock2) {
            try {
                Thread.sleep(1);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }

            list2.add(random.nextInt(100));
        }

    }

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

    public void main() {
        System.out.println("Starting ...");

        long start = System.currentTimeMillis();

        Thread t1 = new Thread(new Runnable() {
            public void run() {
                process();
            }
        });

        Thread t2 = new Thread(new Runnable() {
            public void run() {
                process();
            }
        });

        t1.start();
        t2.start();

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

        long end = System.currentTimeMillis();

        System.out.println("Time taken: " + (end - start));
        System.out.println("List1: " + list1.size() + "; List2: " + list2.size());
    }
}
4
  • 2
    No reason. You should really only ever synchronize on final variables. If you can mark your List as final then you can simply synchronize on it. Using a separate object separates concerns and could be considered to make code clearer. Commented Sep 29, 2014 at 11:42
  • 2
    Agree with Boris. One point I would add it that you never want to lock on a field that is returned by a getter. So by isolating the lock from any field used for a different purpose you eliminate this possibility. Commented Sep 29, 2014 at 11:47
  • Thanks @BoristheSpider and @John B. Why only lock on final variables? Why not using getters? Commented Sep 29, 2014 at 12:10
  • 1
    The JVM will not let more than one thread synchronize on the same object at the same time. Can two threads enter a synchronized(foo) block at the same time? Absolutely! because one thread can enter the block, and then change foo to refer to a different object, and then the other thread comes along and synchronizes on the other object. When that happens it's usually a mistake. Making foo final helps you to avoid that mistake. Commented Sep 29, 2014 at 15:26

1 Answer 1

4

I don't think the motivation for that is expressed in the code you gave, but it is generally a best practice. However, the same best practice demands that the lock objects be final as well.

If the lists in question were either accepted from the outside or exposed to the outside via a method, then the benefit of the separate lock objects becomes more obvious: it is never a good idea to expose your locks to alien code because the alien code can then use them on its own for locking, breaking your own usage pattern.

If the lists are strictly private, then their monitors would be usable for internal locking; however, a later change to the access policy on the lists may inadvertently affect the locking policies. So, starting out with private locks also serves to avoid any future bugs.

Sign up to request clarification or add additional context in comments.

2 Comments

Awesome - it seemed to me that it would make sense to lock on whichever object would be accessed by multiple threads, but I understand your points. Will just creating a new Object work every time? While I'm still learning, it seems that the new object has no relation to the List.
There is no relation between an object and its monitor in any case---it's just another monitor. The only point of contact between the two is the somewhat awkward "convenience" syntax which allows to mark a method as synchronized. That feature actually caused much more confusion than convenience.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.