Fix several node initialization issues #8392

pull sipa wants to merge 3 commits into bitcoin:master from sipa:fixactivatewait changing 4 files +40 −15
  1. sipa commented at 2:08 PM on July 22, 2016: member

    Fixes #8117

    There were several issues:

    • InitBlockIndex was still calling ActivateBestChain (intended so the genesis block would be activated), which caused the full chain activate to occur immediately (before the node was fully started and RPC was enabled). Remove this, as this is dealt with during the background import thread now.
    • Use a signal and a condition variable to make AppInit2 wait for the genesis block to activate, rather than a sleep-and-test loop (which needs the heavily contended cs_main at that point).
    • Introduce an extra init message after loading the banlist (which is confusing).
    • Do a block space check earlier.

    If accepted, I propose that the first commit is backported to 0.13. The second needs extra translation, unfortunately.

  2. sipa commented at 2:17 PM on July 22, 2016: member

    I guess this does not entirely fix #8117: the main problem I was observing is that after a call with -reindex-chainstate, RPC calls would fail with "error code: -28: error message: Loading banlist..." until the full reindex would complete. That should be fixed by this, though fully activating may be delayed still due to the contention between CNode creation and block processing in the background.

  3. jonasschnelli added the label Refactoring on Jul 23, 2016
  4. in src/init.cpp:None in cfa5d1f5f2 outdated 
    1451 | -
1452 | -        if (!fHaveGenesis) {
1453 | -            MilliSleep(10);
1454 | +    {
1455 | +        boost::unique_lock<boost::mutex> lock(cs_GenesisWait);
1456 | +        while (!fHaveGenesis) {
    NicolasDorier commented at 2:41 AM on July 24, 2016:

    I'm not familiar with this code so I may be wrong. However, it seems you are locking cs_GenesisWait waiting for fHaveGenesis to be true, while at the same time, the code responsible to make fHaveGenesis true at https://github.com/bitcoin/bitcoin/pull/8392/files#diff-c865a8939105e6350a50af02766291b7R521 need the same cs_GenesisWait lock. Seems deadlock ?

    sipa commented at 8:22 AM on July 24, 2016:

    @NicolasDorier Waiting on a condition variable releases the lock temporarily.

  5. Use a signal to continue init after genesis activation 0fd2a33648
  6. Add extra message to avoid a long 'Loading banlist' aa59f2ed3f
  7. Do diskspace check before import thread is started 9d4eb9ad99
  8. sipa force-pushed on Jul 30, 2016
  9. sipa commented at 12:19 AM on July 30, 2016: member

    Rebased and fixed a bug in unit tests (it needed a call to ActivateBestChain as well now, since removing it from InitBlockIndex).

  10. sipa cross-referenced this on Jul 30, 2016 from issue Initialization reports "Loading banlist..." while doing something else by laanwj
  11. laanwj commented at 6:34 AM on August 2, 2016: member

    Going to test this one

  12. laanwj commented at 1:02 PM on August 2, 2016: member

    It now does all the catching up at Starting network threads..., which I suppose is better than at Loading banlist... but the underlying problem is still there:

    2016-08-02 12:57:08 init message: Loading banlist...
2016-08-02 12:57:08 Loaded 0 banned node ips/subnets from banlist.dat  47ms
2016-08-02 12:57:08 init message: Starting network threads...
2016-08-02 12:57:08 Added connection peer=0
2016-08-02 12:58:07 Pre-allocating up to position 0x900000 in rev00559.dat
2016-08-02 12:58:07 UpdateTip: new best=000000000000000000b4507ddb1dd656f75cf36889931653f83775e69920963c height=418553 version=0x20000001 log2_work=84.910427 tx=139363153 date='2016-06-29 22:02:20' progress=0.982011 cache=10.7MiB(4325tx)
2016-08-02 12:58:45 UpdateTip: new best=000000000000000000b3fbefdf962686338d9a0b7177adfb21962041f2147b6f height=418554 version=0x20000000 log2_work=84.910463 tx=139363318 date='2016-06-29 21:58:48' progress=0.982010 cache=15.2MiB(6634tx)
2016-08-02 12:59:11 UpdateTip: new best=00000000000000000247db830d97fd6d6ac97e471b5e0e3062a6b66eec02b5a6 height=418555 version=0x30000001 log2_work=84.910499 tx=139364312 date='2016-06-29 22:06:07' progress=0.982013 cache=17.8MiB(10005tx)
2016-08-02 12:59:13 UpdateTip: new best=0000000000000000043fdfe42285486df7ac2192d82aad1e1d240fa9ed4f8eba height=418556 version=0x20000001 log2_work=84.910535 tx=139364477 date='2016-06-29 22:06:05' progress=0.982013 cache=18.4MiB(10401tx)
2016-08-02 12:59:22 UpdateTip: new best=000000000000000003feebd19bb1745ff866e15e3e4f9939ec3dbd5c9f9d6bde height=418557 version=0x30000000 log2_work=84.91057 tx=139365187 date='2016-06-29 22:11:05' progress=0.982014 cache=20.7MiB(11931tx)
2016-08-02 12:59:24 UpdateTip: new best=000000000000000001036a974077ff29582c9504169f286c620d1c86b078b682 height=418558 version=0x20000001 log2_work=84.910606 tx=139365525 date='2016-06-29 22:12:54' progress=0.982015 cache=21.9MiB(12671tx)
2016-08-02 12:59:31 UpdateTip: new best=000000000000000004663f9109b89879caf6d41a671ce17d60c14e6413724df0 height=418559 version=0x20000001 log2_work=84.910642 tx=139366069 date='2016-06-29 22:17:59' progress=0.982017 cache=23.5MiB(13780tx)
2016-08-02 12:59:33 UpdateTip: new best=000000000000000004a15c63f0a457094f608a65f8eab712d33d80baca3922c0 height=418560 version=0x20000001 log2_work=84.910677 tx=139366361 date='2016-06-29 22:20:13' progress=0.982018 cache=23.7MiB(14223tx)
2016-08-02 12:59:33 UpdateTip: new best=000000000000000004867bc3d9e57941bb9df2d883a44d220a255998a0ffcc64 height=418561 version=0x20000001 log2_work=84.910713 tx=139366420 date='2016-06-29 22:20:37' progress=0.982018 cache=23.8MiB(14319tx)
...

    Tried adding a yield in ActivateBestChain. That didn't work (also found this article on the necessity of explicit yields when unlocking). Adding a boost::this_thread::sleep(boost::posix_time::milliseconds(100)); between every block did however work. But that's wasteful...

    Adding a single 200ms sleep before ActivateBestChain in ThreadImport did the trick too. Adding an arbitrary sleep is quite imprecise/fragile though. Could we launch ThreadImport after the network initialization is done? I guess not as-is, as the poll for the genesis block is expected to be present before that. But some other coordination could maybe work.

    In any case ACK on the changes already here.

  13. sipa cross-referenced this on Aug 3, 2016 from issue Store mempool and prioritization data to disk by sipa
  14. laanwj merged this on Aug 4, 2016
  15. laanwj closed this on Aug 4, 2016
  16. laanwj referenced this in commit f97d335942 on Aug 4, 2016
  17. in src/init.cpp:None in 9d4eb9ad99 
    1437 | @@ -1412,26 +1438,20 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
1438 |          BOOST_FOREACH(const std::string& strFile, mapMultiArgs["-loadblock"])
1439 |              vImportFiles.push_back(strFile);
1440 |      }
1441 | +
1442 |      threadGroup.create_thread(boost::bind(&ThreadImport, vImportFiles));
1443 |  
1444 |      // Wait for genesis block to be processed
1445 | -    bool fHaveGenesis = false;
1446 | -    while (!fHaveGenesis && !fRequestShutdown) {
    rebroad commented at 9:58 AM on August 23, 2016:

    This line was only just added in May by Patrick! Is the loss of sensitivity to a shutdown request intentional?

  18. luke-jr referenced this in commit 30eac2d79a on Sep 21, 2016
  19. luke-jr referenced this in commit 3b354d213f on Sep 21, 2016
  20. luke-jr referenced this in commit fc349288cb on Sep 21, 2016
  21. laanwj cross-referenced this on Mar 16, 2017 from issue QT UI never appears by sheldonth
  22. codablock referenced this in commit 0733cb5aa9 on Sep 19, 2017
  23. codablock referenced this in commit 0fc4b45d38 on Dec 29, 2017
  24. codablock referenced this in commit 5e54cf907d on Jan 8, 2018
  25. dagurval cross-referenced this on May 31, 2018 from issue Use non-atomic flushing with block replay by dagurval
  26. lateminer referenced this in commit 6fe2142990 on Oct 24, 2018
  27. andvgal referenced this in commit 9de331388e on Jan 6, 2019
  28. furszy referenced this in commit ebbb876740 on Feb 14, 2020
  29. furszy cross-referenced this on Feb 14, 2020 from issue [Backport] Wait locking for genesis connection. by furszy
  30. furszy referenced this in commit 6bb09adfbb on Feb 17, 2020
  31. wqking referenced this in commit 4130efa05a on Jul 30, 2020
  32. str4d cross-referenced this on Nov 11, 2020 from issue banlist.dat: store banlist on disk by str4d
  33. zkbot referenced this in commit 2d9a9aaa83 on Nov 11, 2020
  34. zkbot referenced this in commit f40121446d on Nov 12, 2020
  35. zkbot referenced this in commit 049951dc45 on Feb 11, 2021
  36. zkbot referenced this in commit b3a6729944 on Feb 16, 2021
  37. zkbot referenced this in commit e85265fbd5 on Feb 17, 2021
  38. zkbot referenced this in commit b4b07a1bbd on Feb 17, 2021
  39. str4d cross-referenced this on Apr 12, 2021 from issue Sync backports by str4d
  40. bitcoin locked this on Sep 8, 2021
Contributors
sipaNicolasDorierlaanwjrebroad
Labels
Refactoring
Linked (view graph)
#8117 Initialization reports "Loading banlist..." while doing something else#8448 Store mempool and prioritization data to disk#10005 QT UI never appears
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:55 UTC