#431 ✓resolved
Martin Kihlgren

[PATCH] extlib has deadlock problems

Reported by Martin Kihlgren | July 2nd, 2008 @ 03:10 AM

In pooling.rb there is at least one case where a deadlock is caused by two threads acquiring the same two locks in different order.

Thread A

acquire lock 1

acquire lock 2

Thread B

acquire lock 2

acquire lock 1

This patch fixes that by making Thread B acquire lock 1 one extra time, before it acquires lock 2:

Thread B

acquire lock 1

acquire lock 2

acquire lock 1

It also makes this solution workable by using Monitor instead of Mutex as lock, which helps since Monitor is reentrant.

Comments and changes to this ticket

  • Sam Smoot

    Sam Smoot July 2nd, 2008 @ 11:50 AM

    • State changed from “new” to “resolved”

    Thanks Martin!


    The only small issue with this is that using Pooling.lock.synchronize is going to use a global lock instead of one local to the current Pool. Which will stall all requests.

    Since only DO is using the pooling right now, and you'll probably have 2 different connections in a worst-case-scenario right now, it's not a big enough issue to keep me from merging it. So good work. But FYI in case you feel like pursuing it further.

  • Martin Kihlgren

    Martin Kihlgren July 2nd, 2008 @ 11:58 AM

    Yes, that is a bit of a pain, but it only happens when you add new Pools, and when you do that you (albeit further down the chain) still need a lock on the Pooling.lock (when you append_pool). Which, of course, was the whole reason this was needed...

    But yes, further work may be needed. But deadlocks are so very painful (I spent several hours hunting this one down...) :(

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

People watching this ticket