Synchronizing static methods

Question

I'm writing code that requires some synchronization between a few static methods. My goal is to block the execution of any of these methods if one of them executes. So for example:

public class Foo{

       static Object lock = new Object();
       static List<SomeObject> listOfObjects = new ArrayList<>() ;


       public static void methodA() {
              synchronized (lock) {
                  //Do some stuff
                    methodB();
              }
       }

       public static List<SomeObject> methodB() {
              synchronized (lock) {
                  //Do some stuff
                   return listOfObjects;
              }
       }

}

Now let's assume that the following is executed from somewhere in the code:

Foo.methodA();
Foo.methodB();

My questions are:

Is it actually synchronized? will methodA and methodB won't run concurrently?
If so, will methodA calling methodB create a deadlock?

This is good synchronization and there won't be a deadlock (since you do not wait for a lock you are already holding). — Islingre, Oct 07 '19 at 22:26
This is synchronized *if* you make that lock final, but this is almost certainly not the best way to implement whatever you're doing. (At a minimum, a singleton-by-use instance is likely to be substantially better.) — chrylis -cautiouslyoptimistic-, Oct 07 '19 at 22:32

Michał Ziober · Accepted Answer · 2019-10-07T23:33:50.720

Answers:

"Is it actually synchronized? will methodA and methodB won't run concurrently?" - yes, these methods are synchronised. More precisely, code blocks in methods are synchronised.
"If so, will methodA calling methodB create a deadlock?" - no, synchronized keyword is re-entrant in nature it means if a synchronized method calls another synchronized method which requires same lock then current thread which is holding lock can enter into that method without acquiring lock" - from 1

You could improve code a little by providing private and final keywords:

private static final Object lock = new Object();

Also, do not return direct reference to a List you want to protect. Return read only view:

return Collections.unmodifiableList(listOfObjects);

Since now, client has only read only access which protects you from many unexpected situations.

score 1 · Answer 2 · answered Oct 07 '19 at 23:09

Is it actually synchronized? will methodA and methodB won't run concurrently?

Yes, this synchronization looks good.
A thread can acquire the same lock multiple times, increasing a lock counter. And after it releases all of them, only then another thread can acquire that lock. In your code, a thread calling methodA() acquires the lock twice, once in methodA() and then in methodB(). Then it releases them twice (when the synchronized blocks end).

If so, will methodA calling methodB create a deadlock?

No, as explained above, it won't create a deadlock unless you are doing some operations in those methods that can take forever to complete, in which case there will be starvation. If you are not doing any such operation, then I don't think you can get a deadlock when you are using only one lock.

WJS · Answer 3 · 2019-10-07T23:27:29.790

Nope. Method A and method B WILL run concurrently because it's not the methods that are synchronized but the code inside. The code will continue normally with whichever has the lock. Then the other method will finish.

    public class Foo1 {

       static Object        lock          = new Object();
       static List<Integer> listOfObjects = new ArrayList<>();

       public static void main(String[] args) {
          new Foo1().start();
       }
       public void start() {
          new Thread(() -> methodA()).start();
          // method A has the lock
          sleep(1000); // sleep 1 second
          // but method B is still entered and prints first
          new Thread(() -> methodB()).start();
       }
       public static void methodA() {
          sleep(5000); // sleep 5 seconds
          System.out.println("Entering methodA()");
          synchronized (lock) {
             // Do some stuff
             methodB();
          }
       }

       public static List<Integer> methodB() {
          System.out.println("Entering methodB()");
          synchronized (lock) {
             // Do some stuff
             return listOfObjects;
          }
       }

       static void sleep(int milli) {
          try {
             Thread.sleep(milli);
          }
          catch (InterruptedException ie) {
          }
       }

    }

Synchronizing static methods

3 Answers3