-
Notifications
You must be signed in to change notification settings - Fork 717
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
Sync Upgrades, part 2. #1336
Sync Upgrades, part 2. #1336
Conversation
Only thing missing here is the cmake new files dependencies, will need to rebase this again on top of #1335 once it's merged to pass travis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code ACK. sync_tests passing nicely both with and without -DDEBUG_LOCKORDER
5395b50
to
cfac9cf
Compare
rebased on top of master after 1335 merge. Ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK cfac9cf
No issue after resync on testnet and mainnet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK cfac9cf
Note: compiling with --enable-debug
will result in a client that won't function properly due to pre-existing potential deadlocks; to be addressed in a future PR.
7e493df Using WAIT_LOCK macro instead of WaitableLock. (furszy) a0d0e33 [Refactor] Complete move from CCriticalSection to identical RecursiveMutex (both are AnnotatedMixin<std::recursive_mutex>). (furszy) 6608757 doc: Add comment to cs_main and mempool::cs (furszy) Pull request description: This was done on top of #1335 and #1336 (Starting in bb575f8). Effort to bring us up up-to-date with upstream's sync.h/cpp sources. This PR contains: * A complete move from CCriticalSection to identical RecursiveMutex (both are AnnotatedMixin<std::recursive_mutex>). * Using WAIT_LOCK macro instead of WaitableLock. ACKs for top commit: random-zebra: utACK 7e493df Fuzzbawls: utACK 7e493df Tree-SHA512: 539fe93566f90246409606acb0aaeb3a5f839110cb96af7868654738685a07b9e1332f8362a04d328825291007d24c80d0b34b1318edc4afe84a8ac8e5affe61
d8f803a [BUG][RPC] Add missing lock in sendrawtransaction (random-zebra) Pull request description: #1336 Introduced a "lock-not-held" assertion in `sendrawtransaction` RPC as per upstream. But, while in Bitcoin `sendrawtransaction` calls `BroadcastTransaction`, which then locks `cs_main` before `AcceptToMemoryPool`, we call directly ATMP without any lock. This fixes it. ACKs for top commit: furszy: utACK d8f803a Fuzzbawls: utACK d8f803a Tree-SHA512: aeac65a86750b0353b7e75d78d145a179d08c2917b664afb9d0bccdef21b09261c9c0b89d5578b2d0ef4a69de3ba8ce0ec73c9562a22c08cf2d971b6176e8647
This was done on top of #1335 (Starts on 7921802). Effort to bring us up-to-date with upstream's sync.h/cpp sources.
The only task that left to be fully up-to-date with upstream, that will leave for a third PR because it touches other areas of the sources and want to keep this as small as possible, is the
CCriticalSection
removal and replacement with theRecursiveMutex
typedef.