zmq: Fix due to invalid argument and multiple notifiers #17445
Merged
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
|
Does this needs backport? |
|
Code review ACK 3e730bf, thanks for adding a test
I guess we could include it in 0.19.1. |
|
This gets rid of the crash, but looking at the log in #17185 (comment) , doesn't this mean |
|
Yeah I forgot to mention that. And I agree it should be done in other PR because maybe it can't be backport. What I could change here is to give a good warning on zmq failure? |
promag
added a commit
to promag/bitcoin
that referenced
this pull request
Dec 23, 2019
Github-Pull: bitcoin#17445 Rebased-From: 3e730bf
Merged
laanwj
added a commit
that referenced
this pull request
Jan 8, 2020
3e730bf zmq: Fix due to invalid argument and multiple notifiers (João Barbosa) Pull request description: ZMQ initialization is interrupted if any notifier fails, and in that case all notifiers are destroyed. The notifier shutdown assumes that the initialization had occurred. This is not valid when there are multiple notifiers and any except the last fails to initialize. Can be tested by running test/functional/interface_zmq.py from this branch with bitcoind from master. Closes #17185. ACKs for top commit: laanwj: Code review ACK 3e730bf, thanks for adding a test Tree-SHA512: 5da710e97dcbaa94896d019e75162d470f6d381ee07c60e5b3e9db93d11e8f7ca9bf2c509efa4486199e88c96c3e720cc96b4e35b62725d4c7db8e8e9bf6e09d
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jan 8, 2020
…tifiers 3e730bf zmq: Fix due to invalid argument and multiple notifiers (João Barbosa) Pull request description: ZMQ initialization is interrupted if any notifier fails, and in that case all notifiers are destroyed. The notifier shutdown assumes that the initialization had occurred. This is not valid when there are multiple notifiers and any except the last fails to initialize. Can be tested by running test/functional/interface_zmq.py from this branch with bitcoind from master. Closes bitcoin#17185. ACKs for top commit: laanwj: Code review ACK 3e730bf, thanks for adding a test Tree-SHA512: 5da710e97dcbaa94896d019e75162d470f6d381ee07c60e5b3e9db93d11e8f7ca9bf2c509efa4486199e88c96c3e720cc96b4e35b62725d4c7db8e8e9bf6e09d
|
Being backported in 17792. |
promag
added a commit
to promag/bitcoin
that referenced
this pull request
Jan 14, 2020
Github-Pull: bitcoin#17445 Rebased-From: 3e730bf
laanwj
added a commit
that referenced
this pull request
Jan 20, 2020
cd67b1d Use correct C++11 header for std::swap() (Hennadii Stepanov) b8101fb Fix comparison function signature (Hennadii Stepanov) eac4907 Don't allow implementers to think ScriptHash(Witness*()) results in nesting computation (Gregory Sanders) e2c45d8 IsUsedDestination shouldn't use key id as script id for ScriptHash (Gregory Sanders) a5489c9 IsUsedDestination should count any known single-key address (Gregory Sanders) 88729d8 Fix issue with conflicted mempool tx in listsinceblock (Adam Jonas) eafcea7 gui: Fix duplicate wallet showing up (João Barbosa) 7e66d04 Drop signal CClientUIInterface::LoadWallet (Russell Yanofsky) 179d55f zmq: Fix due to invalid argument and multiple notifiers (João Barbosa) Pull request description: Backports - #16963 - #17445 - #17258 - #17621 - #17924 - #17634 ACKs for top commit: laanwj: ACK cd67b1d, checked that I got more or less the same result (including conflict resolution) backporting these commits Tree-SHA512: 645786267cfb10a01a56f7cfd91ddead5f1475df5714595ae480237e04d40c5cfb7460b40532279cacd83e4b775a4ace68a258ec2184b8ad0e997a690a9245e5
MarkLTZ
added a commit
to litecoinz-core/litecoinz
that referenced
this pull request
Feb 13, 2020
- [0.19] wallet: Reset reused transactions cache bitcoin#18083 - 0.19: Backports bitcoin#17792 - psbt: handle unspendable psbts bitcoin#17524 - qt: Fix comparison function signature bitcoin#17634 - psbt: check that various indexes and amounts are within bounds bitcoin#17156 - [0.19] psbt: check that various indexes and amounts are within bounds bitcoin#18079 - [0.19] Final backports for 0.19.1 bitcoin#17988 - Bug: IsUsedDestination shouldn't use key id as script id for ScriptHash bitcoin#17924 - qt: Fix deprecated QCharRef usage bitcoin#18101 - gui: Throttle GUI update pace when -reindex bitcoin#18121 - gui: Fix race in WalletModel::pollBalanceChanged bitcoin#18123 - gui: Fix unintialized WalletView::progressDialog bitcoin#18062 - Bugfix: GUI: Hide the HD/encrypt icons earlier so they get re-shown if another wallet is open bitcoin#18007 - bug-fix macos: give free bytes to F_PREALLOCATE bitcoin#17887 - test: add missing #include to fix compiler errors bitcoin#17980 - zmq: Fix due to invalid argument and multiple notifiers bitcoin#17445
jasonbcox
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Sep 3, 2020
Summary: zmq: Fix due to invalid argument and multiple notifiers (João Barbosa) Pull request description: ZMQ initialization is interrupted if any notifier fails, and in that case all notifiers are destroyed. The notifier shutdown assumes that the initialization had occurred. This is not valid when there are multiple notifiers and any except the last fails to initialize. Can be tested by running test/functional/interface_zmq.py from this branch with bitcoind from master. Closes #17185. --- Depends on D7341 Backport of Core [[bitcoin/bitcoin#17445 | PR17445]] Test Plan: ninja ./test/functional/test_runner.py interface_zmq Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7345
sidhujag
pushed a commit
to syscoin-core/syscoin
that referenced
this pull request
Nov 10, 2020
…tifiers 3e730bf zmq: Fix due to invalid argument and multiple notifiers (João Barbosa) Pull request description: ZMQ initialization is interrupted if any notifier fails, and in that case all notifiers are destroyed. The notifier shutdown assumes that the initialization had occurred. This is not valid when there are multiple notifiers and any except the last fails to initialize. Can be tested by running test/functional/interface_zmq.py from this branch with bitcoind from master. Closes bitcoin#17185. ACKs for top commit: laanwj: Code review ACK 3e730bf, thanks for adding a test Tree-SHA512: 5da710e97dcbaa94896d019e75162d470f6d381ee07c60e5b3e9db93d11e8f7ca9bf2c509efa4486199e88c96c3e720cc96b4e35b62725d4c7db8e8e9bf6e09d
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.
ZMQ initialization is interrupted if any notifier fails, and in that case all notifiers are destroyed. The notifier shutdown assumes that the initialization had occurred. This is not valid when there are multiple notifiers and any except the last fails to initialize.
Can be tested by running test/functional/interface_zmq.py from this branch with bitcoind from master.
Closes #17185.