Fix deadlock in recursive datasource registration. #3038
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The datasource cache was taking an exclusive lock on the simple mutex used to protect the singleton's data pointer. This works okay when everyone always calls it non-recursively, but when the recursive flag is true then it will always deadlock when called on any directory with subdirectories. Additionally, many methods which accessed private data members of the cache were not protected by any locks. Since the call pattern of registering datasources is strictly tree-shaped then it's a good candidate for a recursive mutex. This has a slightly higher overhead than a simple mutex, so rather than change the singleton's mutex to be recursive, I've added a new instance mutex to the datasource cache. Also, added a very basic test which reproduces the problem and shows that it's fixed with this patch.
zerebubuth
added a commit
to zerebubuth/vecamole
that referenced
this pull request
Aug 23, 2015
artemp
added a commit
that referenced
this pull request
Aug 25, 2015
…tasource-registration Fix deadlock in recursive datasource registration.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The datasource cache was taking an exclusive lock on the simple mutex used to protect the singleton's data pointer. This works okay when everyone always calls it non-recursively, but when the recursive flag is true then it will always deadlock when called on any directory with subdirectories.
Additionally, many methods which accessed private data members of the cache were not protected by any locks.
Since the call pattern of registering datasources is strictly tree-shaped then it's a good candidate for a recursive mutex. This has a slightly higher overhead than a simple mutex, so rather than change the singleton's mutex to be recursive, I've added a new instance mutex to the datasource cache.
Also, added a very basic test which reproduces the problem (i.e: blocks forever) and shows that it's fixed with this patch.