-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Avoid locking CTxMemPool::cs recursively in CTxMemPool::DynamicMemoryUsage() #19901
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
Conversation
No change in behavior, some call sites already held the lock and a lock is added in the remaining sites.
|
Looks like this is adding a lot of code (complexity). Not sure if that is worth it. |
Since the memory unused by the mempool could be used by the coinscache, |
|
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
This is another step to transit
CTxMemPool::csfromRecursiveMutextoMutex.Split out from #19306. (Along with #19652, #19854 and #19872 it is more than 2/3 of the way to the final goal).
Thread safety annotations, lock assertions, and required explicit locking added. No behavior change.
Please note that now, since #19668 has been merged, it is safe to apply
AssertLockHeld()macros as they do not swallow compile time Thread Safety Analysis warnings.