Skip to content

Reverse lock order to avoid deadlock #256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 3, 2017

Conversation

roelrymenants
Copy link

In class AutorecoveringConnection:

maybeDeleteRecordedAutoDeleteExchange takes the locks on

  • recordedExchanges
  • consumers

maybeDeleteRecordedAutoDeleteQueue takes the locks on

  • recordedQueues
  • consumers

Since the latter one also calls the former, the following deadlock may occur

Thread1:
maybeDeleteRecordedAutoDeleteExchange

  • locked recordedExchanges
  • waiting for consumers (locked by thread2)

Thread2:
maybeDeleteRecordedAutoDeleteQueue

  • locked recordedQueues
  • locked consumers
    maybeDeleteRecordedAutoDeleteExchange
  • waiting for recordedExchanges (locked by thread1)

By reversing the locks, both flows will first lock consumers, avoiding taking other locks
in a different order

In class AutorecoveringConnection:

maybeDeleteRecordedAutoDeleteExchange takes the locks on
- recordedExchanges
- consumers

maybeDeleteRecordedAutoDeleteQueue takes the locks on
- recordedQueues
- consumers

Since the latter one also calls the former, the following deadlock may occur

Thread1:
maybeDeleteRecordedAutoDeleteExchange
- locked recordedExchanges
- waiting for consumers (locked by thread2)

Thread2:
maybeDeleteRecordedAutoDeleteQueue
- locked recordedQueues
- locked consumers
maybeDeleteRecordedAutoDeleteExchange
- waiting for recordedExchanges (locked by thread1)

By reversing the locks, both flows will first lock consumers, avoiding taking other locks
in a different order
@michaelklishin
Copy link
Contributor

Thank you, we are discussing it with @acogoluegnes. Curiously I see that it has been addressed in master but uses a different order.

@michaelklishin michaelklishin merged commit 0cb79fa into rabbitmq:stable Mar 3, 2017
@michaelklishin michaelklishin added this to the 3.6.7 milestone Mar 3, 2017
@roelrymenants roelrymenants deleted the deadlock branch March 3, 2017 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants