How to use synchronized blocks across classes?

Question

I want to know how to use synchronized blocks across classes. What I mean is, I want to have synchronized blocks in more than one class but they're all synchronizing on the same object. The only way that I've thought of how to do this is like this:

//class 1
public static Object obj = new Object();

someMethod(){
     synchronized(obj){
         //code
     }
}


//class 2
someMethod(){
     synchronized(firstClass.obj){
         //code
     }
}

In this example I created an arbitrary Object to synchronize on in the first class, and in the second class also synchronized on it by statically referring to it. However, this seems like bad coding to me. Is there a better way to achieve this?

Sulla

you should gather the both class objects into one new class and, do what you are planning – Borszcz

Maybe you create a singleton class that implements a BlockingQueue and move all synch'ing out of these classes – Meet

Could you guys elaborate? Thanks! – Sulla

Having different classes synchronize on the same object sounds like a design flaw. Re-think your design! – Prepossess

@alfasin why do you think it is a design flaw? – Gorton

@MiserableVariable it implies that different classes are coupled since they're depended on the other classes implementation. That's against encapsulation principle in OO design. – Prepossess

I could design it another way but it would require more methods and more class(s), meaning more overhead. I wanted to know how to do it my way so it would be more readable and have the least amount of method calls and class at objects. – Sulla

@alfasin which is why I was asking in the first place if there was a different way to do it... because it looked like bad code to me. I don't want one class to depend on the other in this way, but I still want synchronization on the same object so different threads can't call those code blocks at the same time. – Sulla

Different threads != different classes – Prepossess

Coupling is not inherently bad. I can see no reason why closely related classes could not use a common lock object. – Gorton

@MiserableVariable synchronization is usually required to preserve consistency of (shared) state that is accessed concurrently. State consistency can be acquired by making sure that invariants are always true, even in a multi-threaded environment. If different classes require the same lock object, it means that invariants are spreaded across these classes. This is usually bad design - even in single-threaded systems. – For

So is there another way to do it? – Sulla

Answer 1

Having a static object that is used as a lock typically is not desirable because only one thread at a time in the whole application can make progress. When you have multiple classes all sharing the same lock that's even worse, you can end up with a program that has little to no actual concurrency.

The reason Java has intrinsic locks on every object is so that objects can use synchronization to protect their own data. Threads call methods on the object, if the object needs to be protected from concurrent changes then you can add the synchronized keyword to the object's methods so that each calling thread must acquire the lock on that object before it can execute a method on it. That way calls to unrelated objects don't require the same lock and you have a better chance of having code actually run concurrently.

Locking shouldn't necessarily be your first go-to technique for concurrency. Actually there are a number of techniques you can use. In order of descending preference:

1) eliminate mutable state wherever possible; immutable objects and stateless functions are ideal because there's no state to protect and no locking required.

2) use thread-confinement where you can; if you can limit state to a single thread then you can avoid data races and memory visibility issues, and minimize the amount of locking.

3) use concurrency libraries and frameworks in preference to rolling your own objects with locking. Get acquainted with the classes in java.util.concurrent. These are a lot better written than anything an application developer can manage to throw together.

Once you've done as much as you can with 1, 2, and 3 above, then you can think about using locking (where locking includes options like ReentrantLock as well as intrinsic locking). Associating the lock with the object being protected minimizes the scope of the lock so that a thread doesn't hold the lock longer than it needs to.

Also if the locks aren't on the data being locked then if at some point you decide to use different locks rather than having everything lock on the same thing, then avoiding deadlocks may be challenging. Locking on the data structures that need protecting makes the locking behavior easier to reason about.

Advice to avoid intrinsic locks altogether may be premature optimization. First make sure you're locking on the right things no more than necessary.

Arching

Good points, I agree it is last resort, but in certain cases slow processing is required. I mean, you want to block the task if other similar task is executing, one similar situation I got in jenkins pipeline to disable concurrent builds across branches. – Tricksy

Answer 2

OPTION 1:

More simple way would be to create a separate object (singleton) using enum or static inner class. Then use it to lock in both the classes, it looks elegant:

// use any singleton object, at it's simplest can use any unique string in double quotes
  public enum LockObj {
    INSTANCE;
  }

  public class Class1 {
    public void someMethod() {
      synchronized (LockObj.INSTANCE) {
        // some code
      }
    }
  }

  public class Class2 {
    public void someMethod() {
      synchronized (LockObj.INSTANCE) {
        // some code
      }
    }
  }

OPTION:2

you can use any string as JVM makes sure it's only present once per JVM. Uniqueness is to make sure no-other lock is present on this string. Don't use this option at all, this is just to clarify the concept.

     public class Class1 {
    public void someMethod() {
      synchronized ("MyUniqueString") {
        // some code
      }
    }
  }

   public class Class2 {
        public void someMethod() {
          synchronized ("MyUniqueString") {
            // some code
          }
        }
      }

Tricksy

Locking on a string literal strikes me as a horrible idea. As far as I know, de-duplication of string constants across classes is an implementation detail and not portable. And there's always the risk that some other developer just happened to synchronize on the same string literal, which could lead to deadlocks or other undesirable behavior. – Roose

Locking on a string here is pretty scary, as @Mike says. Not something I'd advise people to do. – Arching

On use of locking on a String, Some time when people see simple form of objects, concepts are much clear. – Tricksy

Answer 3

Your code seems valid to me, even if it does not look that nice. But please make your Object you are synchronizing on final. However there could be some considerations depending on your actual context.

In any way should clearly state out in the Javadocs what you want to archive.

Another approach is to sync on FirstClass e.g.

synchronized (FirstClass.class) {
// do what you have to do
} 

However every synchronized method in FirstClass is identical to the synchronized block above. With other words, they are also synchronized on the same object. - Depending on the context it may be better.

Under other circumstances, maybe you'd like to prefer some BlockingQueue implementation if it comes down that you want to synchronize on db access or similar.

Oberammergau

synchronized (FirstClass.class) will result in no two threads executing at same time. firstClass.obj will allow multiple threads to execute concurently when using different firstClass objects – Gorton

@MiserableVariable In the question firstClass.obj is static. So it is bound to one thread. – Oberammergau

Google is your friend ;) docs.oracle.com/javase/7/docs/api/java/util/concurrent/… – Oberammergau

Why does it being static make a difference? – Sulla

@Sulla if it is static this one object exists once per jvm, so all threads will synchronize on the same object, if it is not each instance of class will have its sync object (synchronized per instance) – Oberammergau

Well then it has to be static because I want all the threads to synchronize on it. – Sulla

Answer 4

I think what you want to do is this. You have two worker classes that perform some operations on the same context object. Then you want to lock both of the worker classes on the context object.Then the following code will work for you.

public class Worker1 {

    private final Context context;

    public Worker1(Context context) {
        this.context = context;
    }

    public void someMethod(){
        synchronized (this.context){
            // do your work here
        }
    }
}

public class Worker2 {

    private final Context context;

    public Worker2(Context context) {
        this.context = context;
    }

    public void someMethod(){
        synchronized (this.context){
            // do your work here
        }
    }
}


public class Context {

    public static void main(String[] args) {
        Context context = new Context();
        Worker1 worker1 = new Worker1(context);
        Worker2 worker2 = new Worker2(context);

        worker1.someMethod();
        worker2.someMethod();
    }
}

Contrabass

@Sulla If my answer solved your problem, please mark it as the accepted answer. If not you are welcome to comment your problem. – Contrabass

Answer 5

I think you are going the wrong way, using synchronized blocks at all. Since Java 1.5 there is the package java.util.concurrent which gives you high level control over synchronization issues.

There is for example the Semaphore class, which provides does some base work where you need only simple synchronization:

Semaphore s = new Semaphore(1);
s.acquire();
try {
   // critical section
} finally {
   s.release();
}

even this simple class gives you a lot more than synchronized, for example the possibility of a tryAcquire() which will immediately return whether or not a lock was obtained and leaves to you the option to do non-critical work until the lock becomes available.

Using these classes also makes it clearer, what prupose your objects have. While a generic monitor object might be misunderstood, a Semaphore is by default something associated with threading.

If you peek further into the concurrent-package, you will find more specific synchronisation-classes like the ReentrantReadWriteLock which allows to define, that there might be many concurrent read-operations, while only write-ops are actually synchronized against other read/writes. You will find a Phaser which allows you to synchronize threads such that specific tasks will be performed synchronously (sort of the opposite of synchornized) and also lots of data structures which might make synchronization unnecessary at all in certain situations.

All-in-all: Don't use plain synchronized at all unless you know exactly why or you are stuck with Java 1.4. It is hard to read and understand and most probably you are implementing at least parts of the higher functions of Semaphore or Lock.

Amadaamadas

Answer 6

For your scenario, I can suggest you to write a Helper class which returns the monitor object via specific method. Method name itself define the logical name of the lock object which helps your code readability.

public class LockingSupport {
    private static final LockingSupport INSTANCE = new LockingSupport();

    private Object printLock = new Object();
    // you may have different lock
    private Object galaxyLock = new Object();

    public static LockingSupport get() {
        return INSTANCE;
    }

    public Object getPrintLock() {
        return printLock;
    }

    public Object getGalaxyLock() {
        return galaxyLock;
    }
}

In your methods where you want to enforce the synchronization, you may ask the support to return the appropriate lock object as shown below.

public static void unsafeOperation() {
    Object lock = LockingSupport.get().getPrintLock();
    synchronized (lock) {
        // perform your operation
    }
}

public void unsafeOperation2() { //notice static modifier does not matter
    Object lock = LockingSupport.get().getPrintLock();
    synchronized (lock) {
        // perform your operation
    }
}

Below are few advantages:

• By having this approach, you may use the method references to find all places where the shared lock is being used.
• You may write the advanced logic to return the different lock object(e.g. based on caller's class package to return same lock object for all classes of one package but different lock object for classes of other package etc.)
• You can gradually upgrade the Lock implementation to use java.util.concurrent.locks.LockAPIs. as shown below

e.g. (changing lock object type will not break existing code, thought it is not good idea to use Lock object as synchronized( lock) )

public static void unsafeOperation2() {
    Lock lock = LockingSupport.get().getGalaxyLock();
    lock.lock();
    try {
        // perform your operation
    } finally {
        lock.unlock();
    }
}

Hopes it helps.

Leiker

Answer 7

First of all, here are the issues with your current approach:

1. The lock object is not called lock or similar. (Yes ... a nitpick)
2. The variable is not final. If something accidentally (or deliberately) changes obj, your synchronization will break.
3. The variable is public. That means other code could cause problems by acquiring the lock.

I imagine that some of these effects are at the root of your critique: "this seems like bad coding to me".

To my mind, there are two fundamental problems here:

1. You have a leaky abstraction. Publishing the lock object outside of "class 1" in any way (as a public or package private variable OR via a getter) is exposing the locking mechanism. That should be avoided.

2. Using a single "global" lock means that you have a concurrency bottleneck.

The first problem can be addressed by abstracting out the locking. For example:

someMethod() {
     Class1.doWithLock(() -> { /* code */ });
}

where doWithLock() is a static method that takes a Runnable or Callable or similar, and then runs it with an appropriate lock. The implementation of doWithLock() can use its own private static final Object lock ... or some other locking mechanism according to its specification.

The second problem is harder. Getting rid of a "global lock" typically requires either a re-think of the application architecture, or changing to a different data structures that don't require an external lock.

Continuity