Should I try to avoid the static synchronized method

Question

From what I understood, the following code is not efficient:

class Foo { static Resource resource1; static Resource resource2; static synchronized void methodA() { resource1.add("abc"); } static synchronized void methodB() { resource2.add("abc"); } } 

From what I understood, both methods are locked in one object (object of class Foo.class ), so I assume that the next good optimization?

 class Foo { static Resource resource1; static Resource resource2; static void methodA() { synchronized(resource1) { resource1.add("abc"); } } static void methodB() { synchronized(resource2) { resource2.add("123"); } } } 

So far, two resources are independent of each other.

When should you consider using a static synchronized method?

Answer 1

Use the static synchronized construct when your class abstracts access to one critical resource , so class locking is semantically correct.

If your class abstracts access to several critical resources, you need to use a more subtle lock, as in your example.

You can consider the synchronized modifier for methods as syntactic sugar, there is no unnecessary black magic other than locking the class (or instance if the method was not static).

In your first example, it is doubtful why one class provides access to two different critical resources if they are not completely connected. Perhaps you can transfer critical sections to resource classes.

Answer 2

Your optimization is correct.

The first code lock on Foo.class

The second code lock for two different objects: resource1 and resource2 .

Visually you can imagine it

First code:

  Thread 1 Thread 2 ------------------------------ Foo.methodA() Foo.methodB() // A call to methodB needs to wait for completion of methodA 

Second code:

  Thread 1 Thread 2 ------------------------------ Foo.methodA() Foo.methodB() // At the same time in a machine with at least a dual core 

You should use the static synchronized method only when you have one resource for synchronization.

Answer 3

Optimization is good, but be aware of a possible deadlock. For example, someday you decide to access both resources:

 class Foo { static Resource resource1; static Resource resource2; static void methodA() { synchronized(resource1) { resource1.add("abc"); synchronized(resource2) { resource2.add("abc"); } } } static void methodB() { synchronized(resource2) { resource2.add("123"); synchronized(resource1) { resource1.add("123"); } } } } 

This can lead to a dead end:

• Thread Try executing method A ();
• Thread B will try to execute method B () at the same time;
• Thread A get resource lock1, Thread B get resource lock
• Thread Try to get resource lock2 and start waiting for locks for release.
• Thread B will try to obtain a lock on resource1 and start waiting for a lock for release
• Dead end

To avoid this, you can make your resource class a safe thread:

 class Resource { private final Object mLock = new Object(); ... public void add(String str) { synchronized(mLock) { //do stuff } } } 

Answer 4

The second method (locking on objects) is preferable, since it gives you more control when locking. Moreover, this prevents the outside of your class from blocking your class indefinitely, preventing the execution of its own methods.

Consider the following: Imagine that some external code contains the following statements:

  synchronized (Foo.class) { Thread.sleep(10000); } 

Now, if you used synchronization in the methods of the class itself, as in method 1, other classes that are simultaneously trying to call method A or methodB will be blocked until the sleep ends. If, however, you used internal locking of internal objects, as in method 2, then other classes do not have to wait.

Due to the above, I would not recommend method 1.

PS I just noticed that the internal locks in method 2 were not declared final. This will be a problem if they are reassigned when the method is busy locking on them (the lock will then be on another instance). To prevent this, rather declare them final, as shown below:

 final static Resource resource1 = new Resource(...); final static Resource resource2 = new Resource(...); 

In short, never sync with non-final objects.