refactor: use std::shared_mutex & remove Boost Thread #21064

pull fanquake wants to merge 4 commits into bitcoin:master from fanquake:use_std_shared_mutex changing 15 files +28 −217
  1. fanquake commented at 9:04 am on February 2, 2021: member

    This replaces boost::shared_mutex and boost::unique_lock with std::shared_mutex & std::unique_lock.

    Even though some concerns were raised in #16684 with regard to std::shared_mutex being unsafe to use across some glibc versions, I still think this change is an improvement. As I mentioned in #21022, I also think trying to restrict standard library feature usage based on bugs in glibc is not only hard to do, but it’s not currently clear exactly how we do that in practice (does it also extend to patching out use in our dependencies, should we be implementing more runtime checks for features we are using, when do we consider an affected glibc “old enough” not to worry about? etc). If you take a look through the glibc bug tracker you’ll no doubt find plenty of (active) bug reports for standard library code we already using. Obviously not to say we shouldn’t try and avoid buggy code where possible.

    Two other points:

    [Cory mentioned in #21022](/bitcoin-bitcoin/21022/#issuecomment-769274179):

    It also seems reasonable to me to worry that boost hits the same underlying glibc bug, and we’ve just not happened to trigger the right conditions yet.

    Moving away from Boost to the standard library also removes the potential for differences related to Boosts configuration. Boost has multiple versions of shared_mutex, and what you end up using, and what it’s backed by depends on:

    • The version of Boost.
    • The platform you’re building for.
    • Which version of BOOST_THREAD_VERSION is defined: (2,3,4 or 5) default=2. (see here for some of the differences).
    • Is BOOST_THREAD_V2_SHARED_MUTEX defined? (not by default). If so, you might get the “less performant, but more robust” version of shared_mutex.

    A lot of these factors are eliminated by our use of depends, but users will have varying configurations. It’s also not inconceivable to think that a distro, or some package manager might start defining something like BOOST_THREAD_VERSION=3. Boost tried to change the default from 2 to 3 at one point.

    With this change, we no longer use Boost Thread, so this PR also removes it from depends, the build system, CI etc.

    Previous similar PRs were #19183 & #20922. The authors are included in the commits here. Also related to #21022 - pthread sanity checking.

  2. refactor: replace Boost shared_mutex with std shared_mutex in cuckoocache tests
    Co-authored-by: MarcoFalke falke.marco@gmail.com
    Co-authored-by: sinetek pitwuu@gmail.com
    8e55981ef8
  3. refactor: replace Boost shared_mutex with std shared_mutex in sigcache
    Co-authored-by: MarcoFalke falke.marco@gmail.com
    Co-authored-by: sinetek pitwuu@gmail.com
    7097add83c
  4. build: don't build or use Boost Thread 06e1d7d81d
  5. ci: remove boost thread installation
    Adjust fuzzbuzz.yml to only install the Boost components we need.
    060a2a64d4
  6. fanquake added the label Refactoring on Feb 2, 2021
  7. hebasto commented at 9:07 am on February 2, 2021: member
    Concept ACK.
  8. laanwj commented at 11:41 am on February 2, 2021: member
    Code review ACK 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc
  9. practicalswift commented at 12:27 pm on February 2, 2021: contributor
    Concept ACK
  10. DrahtBot commented at 6:41 pm on February 2, 2021: 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:

    • #20744 ([POC] Use std::filesystem. Remove Boost Filesystem & System by fanquake)

    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.

  11. laanwj added this to the milestone 22.0 on Feb 4, 2021
  12. prusnak approved
  13. prusnak commented at 5:38 pm on February 7, 2021: contributor
    utACK
  14. vasild approved
  15. vasild commented at 11:51 am on February 9, 2021: member

    ACK 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc

    Compiles without warnings with Clang 12, the modified cuckoocache_tests/* tests pass.

    For github to pick the Co-authored-by: lines from commit messages, emails should be in <>.

  16. laanwj merged this on Feb 12, 2021
  17. laanwj closed this on Feb 12, 2021

  18. fanquake deleted the branch on Feb 12, 2021
  19. sidhujag referenced this in commit 19506d98a1 on Feb 12, 2021
  20. kittywhiskers referenced this in commit 4c6a1ea73b on Jul 13, 2021
  21. kittywhiskers referenced this in commit e86ba1962e on Jul 15, 2021
  22. kittywhiskers referenced this in commit c38e9297eb on Jul 15, 2021
  23. kittywhiskers referenced this in commit 0286a99e8e on Jul 17, 2021
  24. kittywhiskers referenced this in commit 2a037fc10e on Aug 24, 2021
  25. kittywhiskers referenced this in commit 2ee99baf70 on Sep 11, 2021
  26. kittywhiskers referenced this in commit c9712751fa on Sep 11, 2021
  27. kittywhiskers referenced this in commit 10b58cac05 on Sep 15, 2021
  28. kittywhiskers referenced this in commit 948bce7fb4 on Sep 15, 2021
  29. thelazier referenced this in commit 73b0263ef3 on Sep 19, 2021
  30. thelazier referenced this in commit ce5b83d8c7 on Sep 25, 2021
  31. DrahtBot locked this on Aug 16, 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: 2024-07-05 19:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me