Fixing deadlocks in messy code

I am modifying a little seriously complex code and I need to add my own sync on top of that.

However, the existing code contains about a dozen, if not more, different locks, and my code must call some of its methods. I really don't know the order in which the locks are acquired and I have no control over it.

So my question is what happens if I replace all the locks with one lock ?. From sacrificial granularity, is there another problem I should be aware of?

Thank!

+3


source to share


3 answers


If you change all blocks (and methods) synchronized

and all other blocking structures, I think you should be fine - worst case, your application degenerates to serial execution. But if you only change some of them, you may be stumped. Consider a scenario in which each of two threads acquires multiple locks:

Thread 1:
    synchronized A
        synchronized B

Thread 2:
    synchronized B
        synchronized C

      

There is no risk of deadlock here, but if you replace A

and C

(but not B

) with a new shared lock, then you get:

Thread 1:
    synchronized L
        synchronized B

Thread 2:
    synchronized B
        synchronized L

      



... which is a classic case of deadlock.

Consider another scenario in which locks do not enforce deadlock on their own, but instead block a lock class such as CountDownLatch:

Thread 1:
    synchronized A
        latch L.countDown()

Thread 2:
    synchronized B
        latch L.await()

      

In this case, modifying both blocks synchronized

to lock a shared lock will not result in a deadlock between them, but will result in a deadlock if thread 2 closes the lock first: it will wait for the countDown latch, which will never be because thread 1 is blocked at the entry point synchronized

. This example applies to other locking structures as well: semaphores, blocking queues, etc.

+2


source


I don't think there is any replacement for correct code analysis. The reason this is awful is probably because everyone who had to modify it did exactly what you did and analyzed it carefully.

It's easy enough to write some registration code that should clear up the lock. Once you can cut the layers and have a clear image, it should be relatively easy to replace the entire batch with one modern lock, for example ReadWriteLock

or similar.



You may find it helpful to take advantage of this opportunity to add test code to manage your log in a controlled manner. A very useful addition to any part of complex code.

+1


source


It's hard to tell what will happen without seeing the code. You may run into a lock problem if it tries to capture another lock in one of the locks. It can also slow down the application while waiting for one lock and multiple locks.

0


source







All Articles