net: Fix CThreadInterrupt::sleep_for TSan issues #19029

pull hebasto wants to merge 2 commits into bitcoin:master from hebasto:200520-interrupts changing 2 files +11 −5
  1. hebasto commented at 3:49 pm on May 20, 2020: member

    This PR adds to the ThreadDNSAddressSeed and ThreadOpenAddedConnections threads their own CThreadInterrupt objects, and fixes ThreadSanitizer’s “double lock of a mutex” warnings.

    Fix #19024

  2. in src/net.h:464 in 7bb5707a53 outdated
    459@@ -460,6 +460,8 @@ class CConnman
    460     Mutex mutexMsgProc;
    461     std::atomic<bool> flagInterruptMsgProc{false};
    462 
    463+    CThreadInterrupt g_interrupt_dnsseed_thread;
    464+    CThreadInterrupt g_interrupt_addcon_thread;
    


    MarcoFalke commented at 4:37 pm on May 20, 2020:
    0    CThreadInterrupt m_interrupt_addcon_thread;
    

    Looks like a member to me, no?


    hebasto commented at 4:49 pm on May 20, 2020:
    Sure! Thank you. Updated.
  3. hebasto force-pushed on May 20, 2020
  4. hebasto commented at 4:48 pm on May 20, 2020: member

    Updated 7bb5707a533090e2b1cdc7e27b860b37b25651e1 -> a46122ca6503c780b303d2c8194625faeb8d0f65 (pr19029.01 -> pr19029.02, diff):

    Looks like a member to me, no?

  5. DrahtBot added the label P2P on May 20, 2020
  6. DrahtBot commented at 6:59 pm on May 20, 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:

    • #19084 (net: add comments on dns seed behaviour by ajtowns)
    • #18925 (Add extra thread for scheduler, move TorControl and OpenAddedConnections to scheduler by naumenkogs)
    • #16939 (p2p: Delay querying DNS seeds by ajtowns)
    • #15502 (p2p: Speed up initial connection to p2p network by ajtowns)

    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 7:42 am on May 21, 2020: member

    I’m not sure these warnings aren’t false positive due to the Fedora clang’s bug. See: #19024 (comment)

    So closing for now.

  8. hebasto closed this on May 21, 2020

  9. MarcoFalke commented at 11:49 am on May 21, 2020: member
    An alternative would be to make the mutex recursive for now
  10. hebasto reopened this on May 21, 2020

  11. hebasto commented at 12:27 pm on May 21, 2020: member

    An alternative would be to make the mutex recursive for now

    This won’t compile, unfortunately.

  12. DrahtBot added the label Needs rebase on May 29, 2020
  13. net: Add m_interrupt_dnsseed_thread a68a536c42
  14. net: Add m_interrupt_addcon_thread bad0f26b6c
  15. hebasto force-pushed on May 31, 2020
  16. hebasto commented at 6:41 am on May 31, 2020: member
    Rebased a46122ca6503c780b303d2c8194625faeb8d0f65 -> bad0f26b6c73678c4282343978f75a84b537923a (pr19029.02 -> pr19029.03) due to the conflict with #16939.
  17. DrahtBot removed the label Needs rebase on May 31, 2020
  18. hebasto commented at 2:38 pm on May 31, 2020: member
    Closing for now. See: #19024 (comment)
  19. hebasto closed this on May 31, 2020

  20. DrahtBot 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: 2024-12-19 09:12 UTC

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