Search code examples
javamultithreadingosgideadlockapache-karaf

Thread Deadlock on synchronized Method in OSGi based application (Karaf)


We have an OSGi based application, which offers a central service interface to other bundles, one method of the service imlementation is synchronized:

MyServiceImpl implements Service {

    @Override
    public synchronized doSomething() {

    }

}

Multiple threads access this service (e.g. a camel route and a webservice call) and call the doSomething() method at the same time. So, nothing special here, Java should synchronize the method call here.

Anyway, we face the problem, that our system is stuck: A thread dump tells us, that some thread is in state "Blocked" because another Thread holds a lock on the service implementation object. Each thread is trying to call the doSomething() method and the one thread is waiting forever, that the lock is released.

My question is, how can that happen, here is nothing special, I can not figure out, why the lock is not released !?


Solution

  • Deadlocks should normally no happen with only one lock. The only time you can get a 'deadlock' with one lock is when one thread can monopolize a lock. I.e. it gets it, releases it but grabs it before another thread has a chance. Java synchronized is not a fair lock.

    However, the simplest case of a deadlock is when you have lock A and lock B. A thread T1 gets lock A while simultaneously thread T2 gets lock B. T1 does some work and tries to get lock B. However, B is held by T2 so it blocks. T2 does some work and tries to get lock B. It is locked by T1 so T2 blocks.

    T1 waits for A which is held by T2 and T2 waits for B which is held by T1. So neither can make progress and release their lock.

    So I expect that your doSomething() method is actually getting another lock in its body by calling out to other code.

    There is a common solution and that is to create a list of your locks and when you need multiple locks you always get them in the same order. In that case, no deadlocks can happen. However, the problem is that you can rarely create such a list because today we almost always use code from others.

    For this reason, the synchronized on methods was a bad idea. Synchronized is a low-level construct; you should only use it on very small blocks of code that do not call foreign code. I.e. you update a data structure but don't call out. When you are in a synchronized block and you call other services you are playing Russian roulette. If you need to lock over larger times use the classes in java.util.concurrency that allow timeouts. Timeouts are the simplest solution to deadlocks.

    There are many patterns to properly use synchronized. That said, it is a very complicated area. I learned everything I know about locking (and so much more) from Transaction Processing. A good book about Java concurrency is the Java Concurrency in Practice.