Page MenuHomePhabricator

Give CValidationInterface Support for calling notifications on the CScheduler Thread
ClosedPublic

Authored by deadalnix on Oct 9 2018, 15:23.

Details

Summary
  • Use TestingSetup to DRY qt rpcnestedtests
  • Make ValidationInterface signals-type-agnostic

(by hiding boost::signals stuff in the .cpp)

This allows us to give it a bit more intelligence as we move
forward, including routing some signals through CScheduler. While
the introduction of a "internals" pointer in the class is pretty
ugly, the fact that we no longer need to include boost/signals
directly from validationinterface.h is very much worth the loss.

  • Give CMainSignals a reference to the global scheduler

...so that it can run some signals in the background later

  • Add default arg to CScheduler to schedule() a callback now
  • Support more than one CScheduler thread for serial clients

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.

  • Flush CValidationInterface callbacks prior to destruction

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.

  • Expose if CScheduler is being serviced, assert its not in EmptyQueue

This is a backport of Core PR10179

Test Plan
make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
corepr10179
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3569
Build 5213: Bitcoin ABC Buildbot (legacy)
Build 5212: arc lint + arc unit

Event Timeline

jasonbcox requested changes to this revision.Oct 9 2018, 23:30
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/init.cpp
1814 ↗(On Diff #5366)

This comment blob is nonsensical and probably not part of the PR. I actually made these changes to give some sense to this comment. Please revert this back.

src/scheduler.h
17 ↗(On Diff #5366)

Shouldn't this come before the std/boost includes?

src/test/test_bitcoin.cpp
96 ↗(On Diff #5366)

I can only hope this is true for all of our unit tests...

This revision now requires changes to proceed.Oct 9 2018, 23:30
This revision is now accepted and ready to land.Oct 12 2018, 20:55