fa4652ce5995ace831b6a4d3125bfcac9563ff6f Pass lifetimebound reference to SingleThreadedSchedulerClient (MacroFake)
Pull request description:
Currently a pointer is passed, which is confusing and requires run-time asserts to avoid nullptr dereference.
All call sites can pass a reference, so do that. Also mark it LIFETIMEBOUND to avoid call sites passing a temporary. Also, unrelated cleanup in touched lines.
ACKs for top commit:
pk-b2:
ACK fa4652ce59
jonatack:
Code review ACK fa4652ce5995ace831b6a4d3125bfcac9563ff6f rebased to master, debug build, unit tests
vincenzopalazzo:
ACK fa4652ce59
Tree-SHA512: cd7ec77347e195d659b8892d34c1e9644d4f88552a4d5fa310dc1756eb27050a99d3098b0b0d27f8474230f82c178fd9e22e7018d8248d5e47a7f4caad395e25
2a39ccf1334ef3c48c6f9969a0fc916b9e10aae1 Add include for std::bind. (sinetek)
Pull request description:
Hi, this patch adds in <functional> because the GUI code makes use of std::bind.
That's all.
ACKs for top commit:
jonasschnelli:
utACK 2a39ccf1334ef3c48c6f9969a0fc916b9e10aae1
Tree-SHA512: fb5ac07d9cd5d006182b52857b289a9926362a2f1bfa4f7f1c78a088670e2ccf39ca28214781df82e8de3909fa3e69685fe1124a7e3ead758575839f5f2277a9
fa4632c41714dfaa699bacc6a947d72668a4deef test: Move boost/stdlib includes last (MarcoFalke)
fa488f131fd4f5bab0d01376c5a5013306f1abcd scripted-diff: Bump copyright headers (MarcoFalke)
fac5c373006a9e4bcbb56843bb85f1aca4d87599 scripted-diff: Sort test includes (MarcoFalke)
Pull request description:
When writing tests, often includes need to be added or removed. Currently the list of includes is not sorted, so developers that write tests and have `clang-format` installed will either have an unrelated change (sorting) included in their commit or they will have to manually undo the sort.
This pull preempts both issues by just sorting all includes in one commit.
Please be aware that this is **NOT** a change to policy to enforce clang-format or any other developer guideline or process. Developers are free to use whatever tool they want, see also #18651.
Edit: Also includes a commit to bump the copyright headers, so that the touched files don't need to be touched again for that.
ACKs for top commit:
practicalswift:
ACK fa4632c41714dfaa699bacc6a947d72668a4deef
jonatack:
ACK fa4632c41714dfaa, light review and sanity checks with gcc build and clang fuzz build
Tree-SHA512: 130a8d073a379ba556b1e64104d37c46b671425c0aef0ed725fd60156a95e8dc83fb6f0b5330b2f8152cf5daaf3983b4aca5e75812598f2626c39fd12b88b180
5574e6ed52d6effd3b7beff0f09b44449202a585 refactor: replace RecursiveMutex `m_callbacks_mutex` with Mutex (Sebastian Falbesoner)
3aa258109e3f3e0b1bfc4f811cbadfd6d516208c scripted-diff: rename `m_cs_callbacks_pending` -> `m_callbacks_mutex` (Sebastian Falbesoner)
Pull request description:
This PR is related to #19303 and gets rid of the RecursiveMutex `m_cs_callbacks_pending`. All of the critical sections (6 in total) only directly access the guarded elements, i.e. it is not possible that within one section another one is called, and we can use a regular Mutex:
807169e10b/src/scheduler.cpp (L138-L145)807169e10b/src/scheduler.cpp (L152-L160)807169e10b/src/scheduler.cpp (L169-L172)807169e10b/src/scheduler.cpp (L184-L187)807169e10b/src/scheduler.cpp (L197-L199)807169e10b/src/scheduler.cpp (L203-L206)
Also, it is renamed to adapt to the (unwritten) naming convention to use the `_mutex` suffix rather than the `cs_` prefix.
ACKs for top commit:
hebasto:
ACK 5574e6ed52d6effd3b7beff0f09b44449202a585, I have reviewed the code and it looks OK, I agree it can be merged.
w0xlt:
crACK 5574e6e
Tree-SHA512: ba4b151d956582f4c7183a1d51702b269158fc5e2902c51e6a242aaeb1c72cfcdf398f9ffa42e3072f5aba21a8c493086a5fe7c450c271322da69bd54c37ed1f
This backport does not include changes that depend on bitcoin pr 18037
70a6b529f306ff72ea1badf25e970a92b2b17ab3 lint-cppcheck: Remove -DHAVE_WORKING_BOOST_SLEEP_FOR (Anthony Towns)
294937b39de5924e772f8ed90d35c53290c8acab scheduler_tests: re-enable mockforward test (Anthony Towns)
cea19f685915be8affb2203184a549576194413f Drop unused reverselock.h (Anthony Towns)
d0ebd93270758ea97ea956b8821e17a2d001ea94 scheduler: switch from boost to std (Anthony Towns)
b9c426012770d166e6ebfab27689be44e6e89aa5 sync.h: add REVERSE_LOCK (Anthony Towns)
306f71b4eb4a0fd8e64f47dc008bc235b80b13d9 scheduler: don't rely on boost interrupt on shutdown (Anthony Towns)
Pull request description:
Replacing boost functionality with C++11 stuff.
Motivated by #18227, but should stand alone. Changing from `boost::condition_var` to `std::condition_var` means `threadGroup.interrupt_all` isn't enough to interrupt `serviceQueue` anymore, so that means calling `stop()` before `join_all()` is needed. And the existing reverselock.h code doesn't work with sync.h's DebugLock code (because the reversed lock won't be removed from `g_lockstack` which then leads to incorrect potential deadlock warnings), so I've replaced that with a dedicated class and macro that's aware of our debug lock behaviour.
Fixes#16027, Fixes#14200, Fixes#18227
ACKs for top commit:
laanwj:
ACK 70a6b529f306ff72ea1badf25e970a92b2b17ab3
Tree-SHA512: d1da13adeabcf9186d114e2dad9a4fdbe2e440f7afbccde0c13dfbaf464efcd850b69d3371c5bf8b179d7ceb9d81f4af3cc22960b90834e41eaaf6d52ef7d331
# Conflicts:
# src/reverselock.h
# src/rpc/misc.cpp
# src/scheduler.cpp
# src/scheduler.h
# src/sync.cpp
# src/sync.h
# src/test/reverselock_tests.cpp
# src/test/scheduler_tests.cpp
# src/test/test_dash.cpp
# test/lint/extended-lint-cppcheck.sh
223de8d94d6522f795ec3c2e7db27469f24aa68c Document RNG design in random.h (Pieter Wuille)
f2e60ca98530e0a865ff6c6fd3c5633aec11a515 Use secure allocator for RNG state (Pieter Wuille)
cddb31bb0a132afa50b5350196cf26f0064fe3e2 Encapsulate RNGState better (Pieter Wuille)
152146e782d401aa1ce7d989d62306aabc85f22e DRY: Implement GetRand using FastRandomContext::randrange (Pieter Wuille)
a1f252eda87356fa329c838a7bf569808489648f Sprinkle some sweet noexcepts over the RNG code (Pieter Wuille)
4ea8e50837a0932b31a241988fd68d6730a2048a Remove hwrand_initialized. (Pieter Wuille)
9d7032e4f066777c97c58b1394884716e213790a Switch all RNG code to the built-in PRNG. (Pieter Wuille)
16e40a8b562ad849a5f5e8b21ceb375e46038243 Integrate util/system's CInit into RNGState (Pieter Wuille)
2ccc3d3aa346e96206281a391bc29874cf5ee7f4 Abstract out seeding/extracting entropy into RNGState::MixExtract (Pieter Wuille)
aae8b9bf0f4fd2b801ee72cf191588c8b3a67c3c Add thread safety annotations to RNG state (Pieter Wuille)
d3f54d1c82b131d817b20cd9daa75f9d3c9475e1 Rename some hardware RNG related functions (Pieter Wuille)
05fde14e3afe6f7156ebb6df6cd0e3ae12635b89 Automatically initialize RNG on first use. (Pieter Wuille)
2d1cc5093949f8ea9487a68724162c8b39035ad8 Don't log RandAddSeedPerfmon details (Pieter Wuille)
6a57ca91da23c6a5d91399ffc7fc09a99b6d4c76 Use FRC::randbytes instead of reading >32 bytes from RNG (Pieter Wuille)
Pull request description:
This does not remove OpenSSL, but makes our own PRNG the 'main' one; for GetStrongRandBytes, the OpenSSL RNG is still used (indirectly, by feeding its output into our PRNG state).
It includes a few policy changes (regarding what entropy is seeded when).
Before this PR:
* GetRand*:
* OpenSSL
* GetStrongRand*:
* CPU cycle counter
* Perfmon data (on Windows, once 10 min)
* /dev/urandom (or equivalent)
* rdrand (if available)
* From scheduler when idle:
* CPU cycle counter before and after 1ms sleep
* At startup:
* CPU cycle counter before and after 1ms sleep
After this PR:
* GetRand*:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter
* GetStrongRand*:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter
* /dev/urandom (or equivalent)
* OpenSSL
* CPU cycle counter again
* From scheduler when idle:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter before and after 1ms sleep
* Perfmon data (on Windows, once every 10 min)
* At startup:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter
* /dev/urandom (or equivalent)
* OpenSSL
* CPU cycle counter again
* Perfmon data (on Windows, once every 10 min)
The interface of random.h is also simplified, and documentation is added.
This implements most of #14623.
Tree-SHA512: 0120e19bd4ce80a509b5c180a4f29497d299ce8242e25755880851344b825bc2d64a222bc245e659562fb5463fb7c70fbfcf003616be4dc59d0ed6534f93dd20
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
* Merge #13743: refactor: Replace boost::bind with std::bind
cb53b825c2 scripted-diff: Replace boost::bind with std::bind (Chun Kuan Lee)
2196c51821 refactor: Use boost::scoped_connection in signal/slot, also prefer range-based loop instead of std::transform (Chun Kuan Lee)
Pull request description:
Replace boost::bind with std::bind
- In `src/rpc/server.cpp`, replace `std::transform` with simple loop.
- In `src/validation.cpp`, store the `boost::signals2::connection` object and use it to disconnect.
- In `src/validationinterface.cpp`, use 2 map to store the `boost::signals2::scoped_connection` object.
Tree-SHA512: 6653cbe00036fecfc495340618efcba6d7be0227c752b37b81a27184433330f817e8de9257774e9b35828026cb55f11ee7f17d6c388aebe22c4a3df13b5092f0
* Replace boost::bind with std::bind and remove Boost.Bind includes
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
3ccfa34b32 convert C-style (void) parameter lists to C++ style () (Arvid Norberg)
Pull request description:
In C, an empty parameter list, `()`, means the function takes any arguments, and `(void)` means the function does not take any parameters.
In C++, an empty parameter list means the function does not take any parameters.
So, C++ still supports `(void)` parameter lists with the same semantics, why change to `()`?
1. removing the redundant `void` improves signal-to-noise ratio of the code
2. using `(void)` exposes a rare inconsistency in that a template taking a template `(T)` parameter list, cannot be instantiated with `T=void`
Tree-SHA512: be2897b6c5e474873aa878ed6bac098382cd21866aec33752fe40b089a6331aa6263cae749aba1b4a41e8467f1a47086d32eb74abaf09927fd5a2f44a4b2109a
# Conflicts:
# src/qt/rpcconsole.cpp
97d2b09c12 Add helper to wait for validation interface queue to catch up (Matt Corallo)
36137497f1 Block ActivateBestChain to empty validationinterface queue (Matt Corallo)
5a933cefcc Add an interface to get the queue depth out of CValidationInterface (Matt Corallo)
a99b76f269 Require no cs_main lock for ProcessNewBlock/ActivateBestChain (Matt Corallo)
a734896038 Avoid cs_main in net_processing ActivateBestChain calls (Matt Corallo)
66aa1d58a1 Refactor ProcessGetData in anticipation of avoiding cs_main for ABC (Matt Corallo)
818075adac Create new mutex for orphans, no cs_main in PLV::BlockConnected (Matt Corallo)
Pull request description:
This should fix#11822.
It ended up bigger than I hoped for, but its not too gnarly. Note that "
Require no cs_main lock for ProcessNewBlock/ActivateBestChain" is mostly pure code-movement.
Tree-SHA512: 1127688545926f6099449dca6a4e6609eefc3abbd72f1c66e03d32bd8c7b31e82097d8307822cfd1dec0321703579cfdd82069cab6e17b1024e75eac694122cb
* scripted-diff: Replace #include "" with #include <> (ryanofsky)
-BEGIN VERIFY SCRIPT-
for f in \
src/*.cpp \
src/*.h \
src/bench/*.cpp \
src/bench/*.h \
src/compat/*.cpp \
src/compat/*.h \
src/consensus/*.cpp \
src/consensus/*.h \
src/crypto/*.cpp \
src/crypto/*.h \
src/crypto/ctaes/*.h \
src/policy/*.cpp \
src/policy/*.h \
src/primitives/*.cpp \
src/primitives/*.h \
src/qt/*.cpp \
src/qt/*.h \
src/qt/test/*.cpp \
src/qt/test/*.h \
src/rpc/*.cpp \
src/rpc/*.h \
src/script/*.cpp \
src/script/*.h \
src/support/*.cpp \
src/support/*.h \
src/support/allocators/*.h \
src/test/*.cpp \
src/test/*.h \
src/wallet/*.cpp \
src/wallet/*.h \
src/wallet/test/*.cpp \
src/wallet/test/*.h \
src/zmq/*.cpp \
src/zmq/*.h
do
base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
Signed-off-by: Pasta <pasta@dashboost.org>
* scripted-diff: Replace #include "" with #include <> (Dash Specific)
-BEGIN VERIFY SCRIPT-
for f in \
src/bls/*.cpp \
src/bls/*.h \
src/evo/*.cpp \
src/evo/*.h \
src/governance/*.cpp \
src/governance/*.h \
src/llmq/*.cpp \
src/llmq/*.h \
src/masternode/*.cpp \
src/masternode/*.h \
src/privatesend/*.cpp \
src/privatesend/*.h
do
base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
Signed-off-by: Pasta <pasta@dashboost.org>
* build: Remove -I for everything but project root
Remove -I from build system for everything but the project root,
and built-in dependencies.
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/Makefile.test.include
* qt: refactor: Use absolute include paths in .ui files
* qt: refactor: Changes to make include paths absolute
This makes all include paths in the GUI absolute.
Many changes are involved as every single source file in
src/qt/ assumes to be able to use relative includes.
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/qt/dash.cpp
# src/qt/optionsmodel.cpp
# src/qt/test/rpcnestedtests.cpp
* test: refactor: Use absolute include paths for test data files
* Recommend #include<> syntax in developer notes
* refactor: Include obj/build.h instead of build.h
* END BACKPORT #11651 Remove trailing whitespace causing travis failure
* fix backport 11651
Signed-off-by: Pasta <pasta@dashboost.org>
* More of 11651
* fix blockchain.cpp
Signed-off-by: pasta <pasta@dashboost.org>
* Add missing "qt/" in includes
* Add missing "test/" in includes
* Fix trailing whitespaces
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
64fb0ac Declare single-argument (non-converting) constructors "explicit" (practicalswift)
Pull request description:
Declare single-argument (non-converting) constructors `explicit`.
In order to avoid unintended implicit conversions.
For a more thorough discussion, see ["C.46: By default, declare single-argument constructors explicit"](http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c46-by-default-declare-single-argument-constructors-explicit) in the C++ Core Guidelines (Stroustrup & Sutter).
Tree-SHA512: e0c6922e56b11fa402621a38656d8b1122d16dd8f160e78626385373cf184ac7f26cb4c1851eca47e9b0dbd5e924e39a85c3cbdcb627a05ee3a655ecf5f7a0f1
a56f8b0 Add missing lock in CScheduler::AreThreadsServicingQueue() (Matt Corallo)
Pull request description:
Not an actual bug as this is only used in asserts right now, but
nice to not have a missing lock.
Tree-SHA512: 7e542b150a0be716783e196493d239f2ad15e5376abf54b67d735dc3ef1b10849c090337b849f530c9f7497ddcfb8389b47d64a5dcf6382b7d38838f88cc1100
1f668b6 Expose if CScheduler is being serviced, assert its not in EmptyQueue (Matt Corallo)
3192975 Flush CValidationInterface callbacks prior to destruction (Matt Corallo)
08096bb Support more than one CScheduler thread for serial clients (Matt Corallo)
2fbf2db Add default arg to CScheduler to schedule() a callback now (Matt Corallo)
cda1429 Give CMainSignals a reference to the global scheduler (Matt Corallo)
3a19fed Make ValidationInterface signals-type-agnostic (Matt Corallo)
ff6a834 Use TestingSetup to DRY qt rpcnestedtests (Matt Corallo)
Tree-SHA512: fab91e34e30b080ed4d0a6d8c1214910e383c45440676e37be61d0bde6ae98d61e8903d22b846e95ba4e73a6ce788798350266feba246d8a2ab357e8523e4ac5
888cce5 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo)
399fb8f Add internal method to add new random data to our internal RNG state (Matt Corallo)
Tree-SHA512: 9732a3804d015eaf48d56b60c73880014845fd17a098f1ac2eff6bd50a4beb8b8be65956ac1f0d641e96e3a53c17daddd919401edbf2e3797c3fb687730fb913
Don't clear `stopRequested` and `stopWhenEmpty` at the top of
`serviceQueue`, as this results in a race condition: on systems under
heavy load, some of the threads only get scheduled on the CPU when the
other threads have already finished their work. This causes the flags to
be cleared post-hoc and thus those threads to wait forever.
The potential drawback of this change is that the scheduler cannot be
restarted after being stopped (an explicit reset would be needed), but
we don't use this functionality anyway.
On a busy or slow system, the CScheduler unit test could fail because it
assumed all threads would be done after a couple of milliseconds.
Replace the hard-coded sleep with CScheduler stop() method that
will cleanly exit the servicing threads when all tasks are completely
finished.
