refactor: Replace RecursiveMutex with Mutex in Shutdown() #19180
Conversation
This file contains hidden or 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
|
ACK 1a9ef1d Shutdown is not recursive, so the same thread can never lock twice (UB) |
|
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 was referenced Jun 6, 2020
This was referenced Jun 6, 2020
|
ACK 1a9ef1d verified manually that
I think it should be |
vasild
approved these changes
Jun 8, 2020
|
@sipa Mind reviewing this PR? |
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Jun 8, 2020
…etbase.cpp 78c8f4f refactor: Replace RecursiveMutex with Mutex in netbase.cpp (Hennadii Stepanov) Pull request description: The functions that could lock this mutex, i.e., `{S,G}etProxy()`, `{S,G}etNameProxy()`, `HaveNameProxy()`, `IsProxy()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_proxyinfo_mutex` could be a non-recursive mutex. Related to bitcoin#19180. ACKs for top commit: MarcoFalke: ACK 78c8f4f , reviewed with the -W git option 👮 vasild: ACK 78c8f4f verified that recursion does not happen Tree-SHA512: fc077fb371f38af5d05f1383c6bebf9926167c257892936fefd2d4fe6f679ca40124d25099e09f645d8ec266df222f96c5d0f9fd39eddcad15cbde0b427bc205
maflcko
pushed a commit
that referenced
this pull request
Jun 8, 2020
….cpp cc5c0d2 refactor: Fix formatting of timedata.cpp (Hennadii Stepanov) c2410ce refactor: Replace RecursiveMutex with Mutex in timedata.cpp (Hennadii Stepanov) Pull request description: Only `GetTimeOffset()` and `AddTimeData()` functions lock this mutex. They do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_timeoffset_mutex` could be a non-recursive mutex. Related to #19180. ACKs for top commit: MarcoFalke: ACK cc5c0d2 , checked the second commit with --word-diff-regex=. --ignore-all-space -U0 🦉 vasild: ACK cc5c0d2 verified that recursion is not happening Tree-SHA512: 38f6df689374d4a1a0e9aedb3ed5e885d8285c4da6b75f9bc84ae036936a159ef8276462db33b4f4dd5c71c6312fa9b45380f7a5726959665bc71dc39031be88
This was referenced Jun 8, 2020
maflcko
pushed a commit
that referenced
this pull request
Jun 9, 2020
….cpp bacbfb6 refactor: Replace RecursiveMutex with Mutex in warnings.cpp (Hennadii Stepanov) Pull request description: The functions that could lock this mutex, i.e., `SetMiscWarning()`, `{S,G}etfLargeWorkForkFound()`, `SetfLargeWorkInvalidChainFound()`, `GetWarnings()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_warnings_mutex` could be a non-recursive mutex. Related to #19180. ACKs for top commit: laanwj: Code review ACK bacbfb6 MarcoFalke: ACK bacbfb6 , reviewed with -W --word-diff-regex=. 🎿 Tree-SHA512: cc06d3d30e4051115d176dcfbd496c8562a70087369bccde756c1de42d7dc3f415ef20d3d69ad2599c1d0cd4228d604d7564adc17beac7b6ff92b924b8c20d54
This was referenced Jun 10, 2020
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Mar 1, 2021
Summary: > Step by step, going to replace all of the RecursiveMutex instances with the Mutex ones throughout the code base :) > > Not sure if it is possible in all cases though... > > This one is a low-hanging fruit. This is a backport of [[bitcoin/bitcoin#19180 | core#19180]] Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D9279
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 27, 2021
…arnings.cpp bacbfb6 refactor: Replace RecursiveMutex with Mutex in warnings.cpp (Hennadii Stepanov) Pull request description: The functions that could lock this mutex, i.e., `SetMiscWarning()`, `{S,G}etfLargeWorkForkFound()`, `SetfLargeWorkInvalidChainFound()`, `GetWarnings()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_warnings_mutex` could be a non-recursive mutex. Related to bitcoin#19180. ACKs for top commit: laanwj: Code review ACK bacbfb6 MarcoFalke: ACK bacbfb6 , reviewed with -W --word-diff-regex=. 🎿 Tree-SHA512: cc06d3d30e4051115d176dcfbd496c8562a70087369bccde756c1de42d7dc3f415ef20d3d69ad2599c1d0cd4228d604d7564adc17beac7b6ff92b924b8c20d54
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 28, 2021
…arnings.cpp bacbfb6 refactor: Replace RecursiveMutex with Mutex in warnings.cpp (Hennadii Stepanov) Pull request description: The functions that could lock this mutex, i.e., `SetMiscWarning()`, `{S,G}etfLargeWorkForkFound()`, `SetfLargeWorkInvalidChainFound()`, `GetWarnings()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_warnings_mutex` could be a non-recursive mutex. Related to bitcoin#19180. ACKs for top commit: laanwj: Code review ACK bacbfb6 MarcoFalke: ACK bacbfb6 , reviewed with -W --word-diff-regex=. 🎿 Tree-SHA512: cc06d3d30e4051115d176dcfbd496c8562a70087369bccde756c1de42d7dc3f415ef20d3d69ad2599c1d0cd4228d604d7564adc17beac7b6ff92b924b8c20d54
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 29, 2021
…arnings.cpp bacbfb6 refactor: Replace RecursiveMutex with Mutex in warnings.cpp (Hennadii Stepanov) Pull request description: The functions that could lock this mutex, i.e., `SetMiscWarning()`, `{S,G}etfLargeWorkForkFound()`, `SetfLargeWorkInvalidChainFound()`, `GetWarnings()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_warnings_mutex` could be a non-recursive mutex. Related to bitcoin#19180. ACKs for top commit: laanwj: Code review ACK bacbfb6 MarcoFalke: ACK bacbfb6 , reviewed with -W --word-diff-regex=. 🎿 Tree-SHA512: cc06d3d30e4051115d176dcfbd496c8562a70087369bccde756c1de42d7dc3f415ef20d3d69ad2599c1d0cd4228d604d7564adc17beac7b6ff92b924b8c20d54
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Sep 17, 2021
…arnings.cpp bacbfb6 refactor: Replace RecursiveMutex with Mutex in warnings.cpp (Hennadii Stepanov) Pull request description: The functions that could lock this mutex, i.e., `SetMiscWarning()`, `{S,G}etfLargeWorkForkFound()`, `SetfLargeWorkInvalidChainFound()`, `GetWarnings()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_warnings_mutex` could be a non-recursive mutex. Related to bitcoin#19180. ACKs for top commit: laanwj: Code review ACK bacbfb6 MarcoFalke: ACK bacbfb6 , reviewed with -W --word-diff-regex=. 🎿 Tree-SHA512: cc06d3d30e4051115d176dcfbd496c8562a70087369bccde756c1de42d7dc3f415ef20d3d69ad2599c1d0cd4228d604d7564adc17beac7b6ff92b924b8c20d54
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Sep 19, 2021
…arnings.cpp bacbfb6 refactor: Replace RecursiveMutex with Mutex in warnings.cpp (Hennadii Stepanov) Pull request description: The functions that could lock this mutex, i.e., `SetMiscWarning()`, `{S,G}etfLargeWorkForkFound()`, `SetfLargeWorkInvalidChainFound()`, `GetWarnings()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_warnings_mutex` could be a non-recursive mutex. Related to bitcoin#19180. ACKs for top commit: laanwj: Code review ACK bacbfb6 MarcoFalke: ACK bacbfb6 , reviewed with -W --word-diff-regex=. 🎿 Tree-SHA512: cc06d3d30e4051115d176dcfbd496c8562a70087369bccde756c1de42d7dc3f415ef20d3d69ad2599c1d0cd4228d604d7564adc17beac7b6ff92b924b8c20d54
thelazier
pushed a commit
to thelazier/dash
that referenced
this pull request
Sep 25, 2021
…arnings.cpp bacbfb6 refactor: Replace RecursiveMutex with Mutex in warnings.cpp (Hennadii Stepanov) Pull request description: The functions that could lock this mutex, i.e., `SetMiscWarning()`, `{S,G}etfLargeWorkForkFound()`, `SetfLargeWorkInvalidChainFound()`, `GetWarnings()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_warnings_mutex` could be a non-recursive mutex. Related to bitcoin#19180. ACKs for top commit: laanwj: Code review ACK bacbfb6 MarcoFalke: ACK bacbfb6 , reviewed with -W --word-diff-regex=. 🎿 Tree-SHA512: cc06d3d30e4051115d176dcfbd496c8562a70087369bccde756c1de42d7dc3f415ef20d3d69ad2599c1d0cd4228d604d7564adc17beac7b6ff92b924b8c20d54
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Step by step, going to replace all of the
RecursiveMutexinstances with theMutexones throughout the code base :)Not sure if it is possible in all cases though...
This one is a low-hanging fruit.