Remove unused boost/thread #18758

pull MarcoFalke wants to merge 3 commits into bitcoin:master from MarcoFalke:2004-noBoostThread changing 8 files +6 −15
  1. MarcoFalke commented at 3:39 PM on April 24, 2020: member

    There are predefined interruption points for boost::thread: https://www.boost.org/doc/libs/1_71_0/doc/html/thread/thread_management.html#interruption_points

    However, non-boost threads such as std::thread or the main() thread can obviously not be interrupted. So remove all unused boost/thread from methods that are never executed in a boost::thread.

    Most of them were accompanied by a ShutdownRequested anyway. So even if the current thread was a boost::thread, the interruption point would be redundant. (We only interrupt threads during shutdown)

  2. MarcoFalke added the label Refactoring on Apr 24, 2020
  3. MarcoFalke force-pushed on Apr 24, 2020
  4. hebasto commented at 3:56 PM on April 24, 2020: member

    Concept ACK.

  5. practicalswift commented at 5:53 PM on April 24, 2020: contributor

    Concept ACK

  6. DrahtBot commented at 6:04 PM on April 24, 2020: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #18731 (refactor: Make CCheckQueue RAII-styled by hebasto)
    • #18710 (Add local thread pool to CCheckQueue by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  7. DrahtBot cross-referenced this on Apr 24, 2020 from issue Add local thread pool to CCheckQueue by hebasto
  8. hebasto commented at 2:30 AM on April 27, 2020: member
  9. in src/validation.cpp:4668 in fa8122ceba outdated
    4630 | @@ -4630,8 +4631,6 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi
    4631 |          CBufferedFile blkdat(fileIn, 2*MAX_BLOCK_SERIALIZED_SIZE, MAX_BLOCK_SERIALIZED_SIZE+8, SER_DISK, CLIENT_VERSION);
    4632 |          uint64_t nRewind = blkdat.GetPos();
    4633 |          while (!blkdat.eof()) {
    4634 | -            boost::this_thread::interruption_point();
    4635 | -
    


    hebasto commented at 2:35 AM on April 27, 2020:

    Shouldn't ShutdownRequested() check added to this loop instead?


    MarcoFalke commented at 11:42 AM on May 28, 2020:

    Done in #18786

  10. DrahtBot cross-referenced this on Apr 27, 2020 from issue init: Remove boost from ThreadImport by MarcoFalke
  11. DrahtBot cross-referenced this on Apr 28, 2020 from issue refactor: Make CCheckQueue RAII-styled by hebasto
  12. MarcoFalke added this to the "In progress" column in a project

  13. MarcoFalke marked this as a draft on Apr 29, 2020
  14. laanwj commented at 8:52 AM on April 30, 2020: member

    Concept ACK.

  15. MarcoFalke cross-referenced this on May 26, 2020 from issue wallet: Remove boost from PeriodicFlush by MarcoFalke
  16. MarcoFalke force-pushed on May 28, 2020
  17. MarcoFalke force-pushed on Jun 2, 2020
  18. fanquake commented at 2:51 PM on June 2, 2020: member

    Concept ACK. Not thread related, but could throw in:

    diff --git a/src/init.cpp b/src/init.cpp
    index 37e625129..438dc3762 100644
    --- a/src/init.cpp
    +++ b/src/init.cpp
    @@ -71,9 +71,7 @@
     #include <sys/stat.h>
     #endif
     
    -#include <boost/algorithm/string/classification.hpp>
     #include <boost/algorithm/string/replace.hpp>
    -#include <boost/algorithm/string/split.hpp>
     #include <boost/signals2/signal.hpp>
     #include <boost/thread.hpp>
    

    to remove some more Boost includes.

  19. hebasto commented at 3:24 PM on June 2, 2020: member

    @fanquake

    Concept ACK. Not thread related, but could throw in:

    diff --git a/src/init.cpp b/src/init.cpp
    index 37e625129..438dc3762 100644
    --- a/src/init.cpp
    +++ b/src/init.cpp
    @@ -71,9 +71,7 @@
     #include <sys/stat.h>
     #endif
     
    -#include <boost/algorithm/string/classification.hpp>
     #include <boost/algorithm/string/replace.hpp>
    -#include <boost/algorithm/string/split.hpp>
     #include <boost/signals2/signal.hpp>
     #include <boost/thread.hpp>
    

    to remove some more Boost includes.

    The same cleanup is done in https://github.com/bitcoin/bitcoin/pull/18710/commits/46838e94da7c54389bce1fb7c0951ab338dcc304 from #18710. Mind reviewing it?

  20. MarcoFalke force-pushed on Jun 2, 2020
  21. txindex: Remove unused boost/thread faa958bc28
  22. txdb: Remove unused boost/thread fad8c890f5
  23. refactor: Specify boost/thread/thread.hpp explicitly 89f9fef1f7
  24. MarcoFalke force-pushed on Jun 4, 2020
  25. MarcoFalke marked this as ready for review on Jun 4, 2020
  26. MarcoFalke commented at 2:12 PM on June 4, 2020: member

    Rebased and cherry-picked one commit by @hebasto

  27. hebasto approved
  28. hebasto commented at 3:19 PM on June 4, 2020: member

    ACK 89f9fef1f71dfeff4baa59bc42bc9049a46d911b, tested on Linux Mint 19.3 (x86_64), verified shutdown in different scenarios.

  29. fanquake approved
  30. fanquake commented at 3:01 AM on June 5, 2020: member

    ACK 89f9fef1f71dfeff4baa59bc42bc9049a46d911b

    fad8c890f5ae6e083e416781b4857a1a53ad5249: Checked that the calls to CCoinsViewDB::Upgrade and CBlockTreeDB::LoadBlockIndexGuts only happen in the [init] thread. As mentioned both of these calls were followed by a ShutdownRequested() anyways.

    faa958bc283023334b9377f5fb2210459ca16d82: Checked that txindex is run in a std::thread.

    https://github.com/bitcoin/bitcoin/blob/01b45b2e016f0b0907929e818216edf7157fb03a/src/index/base.cpp#L311-L313

    Also followed by a call to ShutdownRequested().

  31. fanquake merged this on Jun 5, 2020
  32. fanquake closed this on Jun 5, 2020

  33. MarcoFalke deleted the branch on Jun 5, 2020
  34. MarcoFalke commented at 12:43 PM on June 5, 2020: member

    :partying_face:

    Thanks everyone for the reviews on this set! Now all boost thread interruption points are removed:

  35. sidhujag referenced this in commit 45f4aba2bd on Jun 5, 2020
  36. ComputerCraftr referenced this in commit 9ef02aa86c on Jun 16, 2020
  37. fanquake moved this from the "In progress" to the "Done" column in a project

  38. Fabcien referenced this in commit 6197d93261 on May 14, 2021
  39. Bushstar cross-referenced this on Sep 9, 2021 from issue Boost usage reduction by Bushstar
  40. Bushstar cross-referenced this on Sep 10, 2021 from issue C++17 and reduce Boost usage by Bushstar
  41. str4d cross-referenced this on Dec 6, 2021 from issue Backport upstream PRs that remove Boost usage by str4d
  42. bitcoin locked this on Feb 15, 2022

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-20 06:54 UTC