Remove unused boost/thread #18758

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: member

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

   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. 
   hebasto commented at 2:30 am on April 27, 2020: member

   fad6f40af271bd6094a70fea8311e5d0d18e7ae7 how about https://github.com/bitcoin/bitcoin/blob/eef90c14ed0f559e3f6e187341009270b84f45cb/src/wallet/walletdb.cpp#L782-L784 ?
8. 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
9. MarcoFalke added this to the "In progress" column in a project

   ---

10. MarcoFalke marked this as a draft on Apr 29, 2020
11. 
    laanwj commented at 8:52 am on April 30, 2020: member

    Concept ACK.
12. MarcoFalke force-pushed on May 28, 2020
13. MarcoFalke force-pushed on Jun 2, 2020
14. 
    fanquake commented at 2:51 pm on June 2, 2020: member

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

     0diff --git a/src/init.cpp b/src/init.cpp
     1index 37e625129..438dc3762 100644
     2--- a/src/init.cpp
     3+++ b/src/init.cpp
     4@@ -71,9 +71,7 @@
     5 #include <sys/stat.h>
     6 #endif
     7 
     8-#include <boost/algorithm/string/classification.hpp>
     9 #include <boost/algorithm/string/replace.hpp>
    10-#include <boost/algorithm/string/split.hpp>
    11 #include <boost/signals2/signal.hpp>
    12 #include <boost/thread.hpp>
    

    to remove some more Boost includes.

15. 
    hebasto commented at 3:24 pm on June 2, 2020: member

    @fanquake

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

     0diff --git a/src/init.cpp b/src/init.cpp
     1index 37e625129..438dc3762 100644
     2--- a/src/init.cpp
     3+++ b/src/init.cpp
     4@@ -71,9 +71,7 @@
     5 #include <sys/stat.h>
     6 #endif
     7 
     8-#include <boost/algorithm/string/classification.hpp>
     9 #include <boost/algorithm/string/replace.hpp>
    10-#include <boost/algorithm/string/split.hpp>
    11 #include <boost/signals2/signal.hpp>
    12 #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?

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

    Rebased and cherry-picked one commit by @hebasto
23. hebasto approved
24. 
    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.
25. fanquake approved
26. 
    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().

27. fanquake merged this on Jun 5, 2020
28. fanquake closed this on Jun 5, 2020

    ---

29. MarcoFalke deleted the branch on Jun 5, 2020
30. 
    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:

    • #14480
    • #17405
    • #19142
    • #18792
    • #18758
31. sidhujag referenced this in commit 45f4aba2bd on Jun 5, 2020
32. ComputerCraftr referenced this in commit 9ef02aa86c on Jun 16, 2020
33. fanquake moved this from the "In progress" to the "Done" column in a project

    ---

34. Fabcien referenced this in commit 6197d93261 on May 14, 2021
35. DrahtBot locked this on Feb 15, 2022

Contributors
MarcoFalke hebasto practicalswift DrahtBot laanwj fanquake

Labels
Refactoring