waddrmgr/multi: fix scoped mgr reentry deadlock #18
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
This commit resolves a deadlock observed when attempting to generate addresses. There were a few cases, particularly in chainAddressRowToManaged and loadAccountInfo, which accessed the public IsLocked() method of the Manager, even though the shared mutex had already been acquired. The solution is to create an internal isLocked() method, which can be safely called assuming the manager's mutex has already been acquired. As the comments above both of the methods in question specify, we can assume the Manager's mutex *is* already acquired. This commit also reduces some unnecessary code duplication, since the recent changes left both a Locked() and IsLocked() method that perform the same functionality. IsLocked() was favored as it more clearly indicates that the returned value is a boolean.
cfromknecht
added a commit
to cfromknecht/lnd
that referenced
this pull request
Mar 9, 2018
This commit bumps the version of btcwallet used in lnd to incorporate a fix for a reentry deadlock observed during address creation. For more information see: Roasbeef/btcwallet#18
Mehrrun
pushed a commit
to Mehrrun/lnd
that referenced
this pull request
Mar 21, 2018
This commit bumps the version of btcwallet used in lnd to incorporate a fix for a reentry deadlock observed during address creation. For more information see: Roasbeef/btcwallet#18
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.
This commit resolves a deadlock observed when attempting
to generate addresses. There were a few cases, particularly
in
chainAddressRowToManagedandloadAccountInfo, which accessedthe public
IsLocked()method of theManager, even though theshared mutex had already been acquired.
The solution is to create an internal
isLocked()method, whichcan be safely called assuming the
Manager's mutex has already beenacquired. As the comments above both of the methods in question
specify, this is a safe assumption internal to the above methods.
This commit also reduces some unnecessary code duplication, since
the recent changes left both a
Locked()andIsLocked()method thatperform the same functionality.
IsLocked()was favored as it moreclearly indicates that the returned value is a boolean.