[WIP][Validation] Back ports up to "bye bye" boost signal usages. #1973
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
furszy
changed the title
Collaborator
|
Quick note: need to use |
furszy
force-pushed
the
2020_up_to_bye_bye_boost_signal
branch
from
d659143 to
4cf86c5
Compare
Author
|
good catch, updated. |
random-zebra
added a commit
that referenced
this pull request
Dec 17, 2020
…ound 97a85eb [GUI] Connect alert signal (furszy) ce58263 Move GetWarnings() into its own file. (furszy) 3fee808 util GetWarnings, adding "gui" parameter type. (furszy) e074feb Eliminate data races for strMiscWarning and fLargeWork*Found. This moves all access to these datastructures through accessor functions and protects them with a lock. (furszy) 5fdd73e Make QT runawayException call GetWarnings instead of directly access strMiscWarning. (furszy) c684718 Separate CTranslationInterface from CClientUIInterface (furszy) 0b62015 [Cleanup] Remove unused global fields from util.h/cpp (furszy) Pull request description: Back ported two different PRs from upstream to solve possible races over the `strMiscWarning` and `fLargeWork*Found` global fields plus some small cleanup. dashpay#6022: to not depend on `guiinterface.h` every time that a back end text needs translations. bitcoin#9236: eliminating data races for strMiscWarning and fLargeWork*Found and making QT `runawayException` call `GetWarnings("gui")` instead of directly access `strMiscWarning`. Extra information: This work is part of a larger rabbit hole that i'm working on to be able to solve #1973 current issues and compile the project in macOS again (solving the newer boost version errors). ACKs for top commit: Fuzzbawls: ACK 97a85eb random-zebra: utACK 97a85eb and merging... Tree-SHA512: 0a2549f19ecadff1c3e275649599e9cf043404a213f8418e85cc009d7f56c98c704de53f3700a88acd9cdaf1e5ff73bc3b1d9951d76469c7283d2e7cfaaf6628
…interfaces` -BEGIN VERIFY SCRIPT- git mv src/interface src/interfaces git grep -l "interface/" | xargs sed -i "s,interface/,interfaces/,g"; -END VERIFY SCRIPT-
This allows doing some of the steps before e.g. daemonization and some fater. Coming from btc@0cc8b6bc44bea29e24fa4e13d8a9bbe4f1483680
Before daemonization, just probe the data directory lock and print an
early error message if possible.
After daemonization get the data directory lock again and hold on to it until exit
This creates a slight window for a race condition to happen, however this condition is harmless: it
will at most make us exit without printing a message to console.
$ src/pivxd -testnet -daemon
PIVX server starting
$ src/pivxd -testnet -daemon
Error: Cannot obtain a lock on data directory /home/orion/.pivx/testnet4. PIVX Core is probably already running.
Coming from btc@16ca0bfd2848424de7deae307283d9eb9de8a978
…g +1.74 boost deployment issues)
(by hiding boost::signals stuff in the .cpp) coming from btc@3a19fed9db558a5f666d965b6f602f7faf74ab73
…run some signals in the background later coming from btc@cda1429d5bfee129a0d1f6f1c65962b30251bafb
This will be used by CValidationInterface soon. This requires a bit of work as we need to ensure that most of our callbacks happen in-order (to avoid synchronization issues in wallet) - we keep our own internal queue and push things onto it, scheduling a queue-draining function immediately upon new callbacks.
Note that the CScheduler thread cant be running at this point, it has already been stopped with the rest of the init threadgroup. Thus, just calling any remaining loose callbacks during Shutdown() is sane.
PR bitcoin#10286 introduced a few steps which are not robust to early shutdown in initialization. Stumbled upon this with bitcoin#11781, not sure if there are other scenarios that can trigger it, but it's harden against this in any case.
…nge-based loop instead of std::transform coming from btc@2196c51821e340c9a9d2c76c30f9402370f84994
furszy
force-pushed
the
2020_up_to_bye_bye_boost_signal
branch
2 times, most recently
from
de1dfde to
3079e35
Compare
-BEGIN VERIFY SCRIPT-
for j in $(seq 1 6)
do
sed -i "s/ boost::placeholders::_${j}/ std::placeholders::_${j}/g" $(git grep --name-only " boost::placeholders::_${j}" -- '*.cpp' '*.h')
done
sed -i "s/boost::bind/std::bind/g" $(git grep --name-only boost::bind -- '*.cpp' '*.h')
sed -i "s/boost::ref/std::ref/g" $(git grep --name-only boost::ref -- '*.cpp' '*.h')
sed -i '/boost\/bind/d' $(git grep --name-only boost/bind)
-END VERIFY SCRIPT-
When a wallet is created it is registered in the validation interface (in CWallet::CreateWalletFromFile) but it is not immediately added to the wallets list. If a shutdown is requested before AddWallet (case more evident when -rescan is set) then m_internals can be released (in Shutdown -> UnregisterBackgroundSignalScheduler) before the wallet and then ReleaseWallet would call UnregisterValidationInterface with m_internals already released.
furszy
force-pushed
the
2020_up_to_bye_bye_boost_signal
branch
from
3079e35 to
c5c8e04
Compare
Author
|
Reworked the PR one more time, added a good number of changes, should be ready to go now (long way to get to this point..). |
Not an actual bug as this is only used in asserts right now, but nice to not have a missing lock.
furszy
added a commit
that referenced
this pull request
Jan 16, 2021
30fcd32 Check m_internals in UnregisterValidationInterface (João Barbosa) 2e3a32c scripted-diff: Replace boost::bind with std::bind (furszy) ecdb75a Shutdown process: verify peerLogic and g_connman existence before try to accessing them. (furszy) b5b9f99 refactor: Use boost::scoped_connection in signal/slot, also prefer range-based loop instead of std::transform (furszy) adbd8bb prefer range-based loop instead of std::transform (furszy) eb70819 Make ValidationInterface signals-type-agnostic (furszy) 7ad2adf GUI: Event handler generic interface. (furszy) 06494f9 Migrating signal placeholders to correct boost naming scheme. (solving +1.74 boost deployment issues) (furszy) 20c9274 init: Try to aquire datadir lock before and after daemonization Before daemonization, just probe the data directory lock and print an early error message if possible. (furszy) b98cb09 init: Split up AppInit2 into multiple phases (furszy) a2578de scripted-diff: Avoid `interface` keyword. Rename `interface` dir to `interfaces` (furszy) Pull request description: A set of commits decoupled from #1973, solving #1840. The final goal is #1973 but the scope there got pretty big and is requiring more previous back ports (like 9725 among others that are an entire branch of back ports by themselves) so.. better to start merging this sooner rather than later to not continue having issues on macOS deployment (or well, more specifically issues with newer boost versions). ACKs for top commit: random-zebra: ACK 30fcd32 Fuzzbawls: ACK 30fcd32 Tree-SHA512: f24ee57d58b717e254263c97f9d0e2e42b996bcc25d1baa270c893fa270340335d7a29b82c8246e1e9ab17c2c743479e1176a793006167c600b400b649ecbfdd
Author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
A second attempt to solve #1840 in a more cleaner way.
Made my own way up to remove most of the the boost signal usages. Had to adapt some PRs from upstream with further customizations as we aren't one to one with them.
Base upstream PRs are the following ones: