Java synchronize on object

Question

How to synchronize two different methods from the same class in order to lock on the same object? Here is an example:

public class MyClass extends Thread implements Observer{
  public List<AnotherClass> myList = null;
  
  public MyClass(List<AnotherClass> myList){
    this.myList = myList;
  }
  
  public void run(){
    while(true){
       //Do some stuff 
       myList.add(NotImportantElement);
    }
  }

  public void doJob{
    for(int i=0; i<myList.size; i++){
      ElementClass x = myList.get(i);
      //Do some more stuff
    }
  }
}

The question is how can I stop run() from accesing myList when doJob is executed and viceversa?

Imagine this: I start the thread and start adding elements to my list. At a random moment I call doJob() from another class that holds a reference to my thread.

How should I do the lock? Thanks!

L.E.

Ok, I understood the concept of the lock, but now I have another question.

Suppose I have a class with public static myList and only one instance of that class. From that instance I create n instances of Thread that take every element of that list and do some stuff with it.

Now, at a specific moment, myList is updated. What happens with those Threads that already were processing myList elements? How should I lock access on myList while updating it?

Phasis

@Grammin I would say that it doesn't matter. – Impedance

If you have another question, your best bet is to open another post. That way it will be seen. – Overeat

Answer 1

NOTE: This code assumes you only have one instance of MyClass. according to your post that sounds like the case.

public class MyClass extends Thread implements Observer{
  private List<AnotherClass> myList = null;
  private Object lock = new Object();

  public MyClass(List<AnotherClass> myList){
    this.myList = new ArrayList(myList);
  }

  public void run(){
    while(true){
       //Do some stuff 
       synchronized(lock) {
        myList.add(NotImportantElement);
       }
    }
  }

  public void doJob{
    synchronized(lock) {
      for(int i=0; i<myList.size; i++){
        ElementClass x = myList.get(i);
        //Do some more stuff
      }
    }
  }
}

EDIT: Added making a copy of List so that external entities could not change the list as per JB Nizet

EDIT 2: Made variables private so nobody else can access them

Overeat

There is a flaw, though : since myList comes from the outside, there is no guarantee that another thread accesses myList directly, bypassing the lock. The constructor should do a defensive copy of the list it receives. – Caoutchouc

@JB Nizet Thanks for pointing that out - I fixed the sample. Also made instance vars private – Overeat

What's wrong with just "synchronized"? That locks on the instance, which would seem to do exactly the same with one fewer objects. – Tatting

@Tatting : he would still need a synchronized(this) inside the run method, because if he puts the synchronized on the method, the lock will never be released : it's an infinite loop. – Caoutchouc

Answer 2

You can:

1. Declare both run and doJob synchronized. This will use this as lock;
2. Declare list as final and synchronize on it. This will use list as lock. Declaring lock field as final is good practice. This way some methods of your class my synchronize on one object, while other methods can use other object for synchronization. This reduces lock contention but increases code complexity;
3. Introduce explicit java.util.concurrent.locks.Lock variable and use it's methods for synchronization. This will improve code flexibility, but will increase code complexity as well;
4. Don't do explicit synchronization altogether and instead employ some thread-safe data structure from JDK. For example, BlockingQueue or CopyOnWriteArrayList. This will reduce code complexity and ensure thread safety.
5. Employ synchronization by reads/writes to volatile field. See this SO post. This will ensure safety, but will increase complexity greatly. On the second thought, don't do this :)

Alkanet

Upvoted for (4.): you'll avoid a lot of trouble by using a concurrent list. – Sealskin

When run executes it will be synchronized and will never release the lock - #1 will not work. #2 has the issue that some other object can operate on the List and that would need to be synchronized also. – Overeat

Answer 3

You can either add

synchronized

keyword to both methods OR use the

synchronized(Myclass.class) {
}

The former essentially uses the Myclass.class object but it is not as fine-grained as the latter.

Kruter

Answer 4

Declare both methods as synchronized to lock every instance, or use a synchronized(this){...} block to make the lock only on the current instance.

Taveda

Answer 5

synchronized(myList) {
    // do stuff on myList
}

Specific documentation: Intrinsic Locks and Synchronization

Yet I encourage you to use a thread-safe concurrent data structure for what you want to achieve to avoid doing synchronizing yourself and to get (a lot) better performance: Concurrent package summary

Redress