Clean up lockorder data of destroyed mutexes #7846

pull sipa wants to merge 1 commits into bitcoin:master from sipa:cleanlocks changing 2 files +65 −23
  1. sipa commented at 9:37 PM on April 8, 2016: member

    The lockorder potential deadlock detection works by remembering for each lock A that is acquired while holding another B the pair (A,B), and triggering a warning when (B,A) already exists in the table.

    A and B in the above text are represented by pointers to the CCriticalSection object that is acquired. This does mean however that we need to clean up the table entries that refer to any critical section which is destroyed, as its memory address can potentially be used for another unrelated lock in the future.

    Implement this clean up by remembering not only the pairs in forward direction, but also backward direction. This allows for fast iteration over all pairs that use a deleted CCriticalSection in either the first or the second position.

  2. sipa commented at 9:37 PM on April 8, 2016: member

    Hopefully this fixes #7470.

  3. laanwj added the label Tests on Apr 9, 2016
  4. laanwj commented at 7:49 AM on April 9, 2016: member

    Thanks! I lack the understanding of this code to be able to review it, unfortunately this makes the code even more complicated. Luckily it's only active in debug mode (--enable-debug) anyhow.

    utACK if it fixes #7470.

  5. Clean up lockorder data of destroyed mutexes
    The lockorder potential deadlock detection works by remembering for each
    lock A that is acquired while holding another B the pair (A,B), and
    triggering a warning when (B,A) already exists in the table.
    
    A and B in the above text are represented by pointers to the CCriticalSection
    object that is acquired. This does mean however that we need to clean up the
    table entries that refer to any critical section which is destroyed, as it
    memory address can potentially be used for another unrelated lock in the future.
    
    Implement this clean up by remembering not only the pairs in forward direction,
    but also backward direction. This allows for fast iteration over all pairs that
    use a deleted CCriticalSection in either the first or the second position.
    5eeb913d6c
  6. sipa force-pushed on Apr 10, 2016
  7. sipa commented at 12:31 PM on April 10, 2016: member

    @laanwj Added a PR and commit description that explains the change and rationale.

  8. jonasschnelli commented at 2:06 PM on April 11, 2016: contributor

    Nice! LGTM. utACK 5eeb913d6cff9cfe9a6769d7efe4a7b9f23de0f4.

  9. MarcoFalke cross-referenced this on Apr 12, 2016 from issue getblockchaininfo: make bip9_softforks an object, not an array. by rustyrussell
  10. laanwj commented at 1:07 PM on April 14, 2016: member

    @sipa OHH I get it now. So sync.cpp keeps track of locks after they are unlocked. They are simply never cleaned up. I had assumed this was only keeping track of active locks, so I didn't understand why locks would be deleted that were still held somewhere. But well in that case this fixes a memory leak too! tACK 5eeb913

  11. laanwj merged this on Apr 14, 2016
  12. laanwj closed this on Apr 14, 2016

  13. laanwj referenced this in commit 491171f929 on Apr 14, 2016
  14. laanwj cross-referenced this on Apr 14, 2016 from issue Potential deadlock assertion in net code on 0.12RC3 by GIJensen
  15. daira cross-referenced this on Oct 21, 2016 from issue Upstream: Clean up lockorder data of destroyed mutexes by bitcartel
  16. sipa cross-referenced this on Jan 23, 2017 from issue locking inconsistencies in the network code by dooglus
  17. sipa cross-referenced this on Jan 23, 2017 from issue Potential deadlock on {cs_main, pnode->cs_vSend} by ashleyholman
  18. thokon00 referenced this in commit ee13b62adb on Mar 20, 2017
  19. codablock referenced this in commit 97a0d6676e on Sep 16, 2017
  20. codablock referenced this in commit 187d8b1711 on Sep 19, 2017
  21. codablock referenced this in commit f1f3fa327a on Dec 20, 2017
  22. dustinengle cross-referenced this on Jun 21, 2018 from issue Dev update and fixes by dustinengle
  23. LarryRuane referenced this in commit 41713465f5 on Feb 20, 2021
  24. LarryRuane cross-referenced this on Feb 20, 2021 from issue Bitcoin 0.13 locking PRs by LarryRuane
  25. zkbot referenced this in commit d95a957841 on Apr 1, 2021
  26. zkbot referenced this in commit 2d3b58c993 on Apr 1, 2021
  27. bitcoin locked this on Sep 8, 2021

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