Prevent data race for pathHandlers
#25983
Merged
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
|
Concept and initial-look approach ACK |
hebasto
added a commit
to hebasto/bitcoin
that referenced
this pull request
Sep 7, 2022
Github-Pull: bitcoin#25983 Rebased-From: 4296dde
|
Backported into 23.x in #26033. |
hebasto
added a commit
to hebasto/bitcoin
that referenced
this pull request
Sep 7, 2022
Github-Pull: bitcoin#25983 Rebased-From: 4296dde
|
Backported into 22.x in #26034. |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Sep 7, 2022
4296dde Prevent data race for `pathHandlers` (Hennadii Stepanov) Pull request description: Fixes bitcoin#19341. ACKs for top commit: ryanofsky: Code review ACK 4296dde. This should protect the vector. It also seems to make the http_request_cb callback single threaded, but that seems ok, since it is just adding work queue items not actually processing requests. Tree-SHA512: 1c3183100bbc80d8e83543da090b8f4521921cf30d444e3e4c87102bf7a1e67ccc4dfea7e9990ac49741b2a5708f259f4eced9d4049c20ae4e531461532a6aef
maflcko
pushed a commit
that referenced
this pull request
Oct 24, 2022
2c6c628 Prevent data race for `pathHandlers` (Hennadii Stepanov) Pull request description: Backport of #25983 to the 22.x branch. ACKs for top commit: dergoegge: ACK 2c6c628 Tree-SHA512: 9b172f73407fdd5a79e67ed1b2b3b7c6e7989ea1de6757c839ee7040d62ebbd87d10770c6fcb39709a07d52345dc9b7b244ef2b490c9ad8a656ff8ad4d618a01
maflcko
pushed a commit
that referenced
this pull request
Oct 24, 2022
38d4601 Prevent data race for `pathHandlers` (Hennadii Stepanov) Pull request description: Backport of #25983 to the 23.x branch. ACKs for top commit: dergoegge: ACK 38d4601 Tree-SHA512: b235d6d2cb374baf1b54c09f4cd2feca7b6c1588d081532e987fd5def8ed0dee4b8255112b130a77aca633ec6a63cfd81f215b2e7a403c213eb6048a54774d26
vertiond
added a commit
to vertcoin-project/vertcoin-core
that referenced
this pull request
Nov 3, 2022
* rpc: Capture potentially large UniValue by ref for rpcdoccheck Github-Pull: 25237 Rebased-From: 20ff499 * Add pure Python RIPEMD-160 Github-Pull: 23716 Rebased-From: ad3e9e1 * Swap out hashlib.ripemd160 for own implementation Github-Pull: 23716 Rebased-From: 5b559dc * windeploy: Renewed windows code signing certificate Github-Pull: #25201 Rebased-From: 7e9fe6d * Prevent data race for `pathHandlers` Github-Pull: bitcoin/bitcoin#25983 Rebased-From: 4296dde * Revert "build: Use Homebrew's sqlite package if it is available" This reverts ee7b84e from #20527. This change was made without any rationale, maybe other than a brew installed version might be newer, and that's "better". However when building from source on macOS, it just results in drastically worse perofrmance, and results in issues / confusions like #25724. Resolves the "build from source" portion of #25724. Building from depends is still not ideal, however I have some other changes that might help improve things in that case. The difference in performance can be observed using the example from bitcoin/bitcoin#25724 (comment), but minified to only 10 descriptors. i.e: ```bash time src/bitcoin-cli createwallet speedy true time src/bitcoin-cli importdescriptors '[ {"desc":"raw(00145846369f3d6ba366d6f5a903fb5cf4dca3763c0e)#k9wh6v62","timestamp":"now"}, {"desc":"raw(001420800aabf13f3a4c4ce3ce4c66cecf1d17f21a6e)#6m0hlfh4","timestamp":"now"}, {"desc":"raw(0014c6bf9715e06d73ebf9b3b02d5cc48d24d8bbabc1)#wyavh36r","timestamp":"now"}, {"desc":"raw(00141ba7807b3f46af113beaea5c698428ce7138cd8a)#jctdsups","timestamp":"now"}, {"desc":"raw(00140c1bd27f10fff01b36ddf3c1febaa1acff19b080)#9s6nc3pk","timestamp":"now"}, {"desc":"raw(00141226e31987e4bc2e63c0ee12908f675e40464b20)#9pp7qm39","timestamp":"now"}, {"desc":"raw(0014f73f149f7503960a5e849c6ee7a8a8c336f631cb)#qtkxv9fc","timestamp":"now"}, {"desc":"raw(0014c8ccb4d81ffc769fc5fdd8d7eed69b0e0cae5749)#hn39qayv","timestamp":"now"}, {"desc":"raw(001498565aead2d67a22a6021d55210f2a917fc22169)#6ar3vwsx","timestamp":"now"}, {"desc":"raw(001403013248ac0cd9eabe176cad162cda2a19f771e1)#4m47mukd","timestamp":"now"} ]' ``` Running master, when building from souce and using brew installed sqlite, this takes ~3.4s. With this PR, the same operation takes ~0.3s. Github-Pull: #25985 Rebased-From: d216d71 * doc: remove brew install sqlite from macOS docs * Adjust `.tx/config` for new Transifex CLI The old Transifex Command-Line Tool is considered deprecated (as of January 2022) and will sunset on Nov 30, 2022. See: https://github.com/transifex/cli/blob/devel/README.md#migrating-from-older-versions-of-the-client An accompanying PR: bitcoin-core/bitcoin-maintainer-tools#142 Github-Pull: #26321 Rebased-From: d6adbb7 * rpc: fix crash in deriveaddresses when derivation index is 2147483647 2147483647 is the maximum positive value of a signed int32, and - currently - the maximum value that the deriveaddresses bitcoin RPC call accepts as derivation index due to its input validation routines. Before this change, when the derivation index (and thus range_end) reached std::numeric_limits<int_32_t>::max(), the "i" variable in the for cycle (which is declared as int, and as such 32 bits in size on most platforms) would be incremented at the end of the first iteration and then warp back to -2147483648. This caused SIGABRT in bitcoind and a core dump. This change assigns "i" an explicit size of 64 bits on every platform, sidestepping the problem. Fixes #26274. Github-Pull: #26275 Rebased-From: addf9d6 * rpc: add non-regression test about deriveaddresses crash when index is 2147483647 This test would cause a crash in bitcoind (see #26274) if the fix given in the previous commit was not applied. Github-Pull: #26275 Rebased-From: 9153ff3 * build: Bump version to 22.1.0rc1 * doc: Update manual pages for 22.1.0rc1 * doc: update version number in bips.md to v22.1 * qt: 22.1rc1 translations update * vtc review: missed branding items Co-authored-by: Martin Zumsande <[email protected]> Co-authored-by: Pieter Wuille <[email protected]> Co-authored-by: laanwj <[email protected]> Co-authored-by: MacroFake <[email protected]> Co-authored-by: Andrew Chow <[email protected]> Co-authored-by: Hennadii Stepanov <[email protected]> Co-authored-by: fanquake <[email protected]> Co-authored-by: muxator <[email protected]>
psgreco
pushed a commit
to psgreco/elements
that referenced
this pull request
Dec 7, 2022
Github-Pull: bitcoin/bitcoin#25983 Rebased-From: 4296dde (cherry picked from commit 2c6c628)
psgreco
pushed a commit
to psgreco/elements
that referenced
this pull request
Dec 7, 2022
Github-Pull: bitcoin/bitcoin#25983 Rebased-From: 4296dde (cherry picked from commit 2c6c628)
psgreco
pushed a commit
to psgreco/elements
that referenced
this pull request
Dec 7, 2022
Github-Pull: bitcoin/bitcoin#25983 Rebased-From: 4296dde (cherry picked from commit 2c6c628)
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.
Fixes #19341.