threading: use correct mutex name in reverse_lock fatal error messages #32829

1. 
   kevkevinpal commented at 7:41 pm on June 29, 2025: contributor

   “Now that REVERSE_LOCK requires the name of the actual mutex, it can be used for better error messages.” - theuni

   This is a follow-up to this comment #32465 (comment)

   I just cherry-picked the commit 85c2848eb575f4abaa81fdd4e8f3b2048693dd98

2. 
   DrahtBot commented at 7:41 pm on June 29, 2025: contributor

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

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32829.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	theuni, TheCharlatan

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   LLM Linter (✨ experimental)

   Possible typos and grammar issues:

   • mutex2 was not most recent critical section locked -> mutex2 was not the most recent critical section locked [missing article “the”]

   drahtbot_id_4_m

3. DrahtBot added the label CI failed on Jun 29, 2025
4. 
   DrahtBot commented at 8:44 pm on June 29, 2025: contributor

   🚧 At least one of the CI tasks failed. Task TSan, depends, gui: https://github.com/bitcoin/bitcoin/runs/45008376829 LLM reason (✨ experimental): The CI failure is due to the test “reverselock_tests” failing.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

5. DrahtBot removed the label CI failed on Jun 29, 2025
6. fanquake requested review from theuni on Jun 30, 2025
7. 
   kevkevinpal commented at 8:23 pm on June 30, 2025: contributor

   So one thing I noticed is that the tests passed, but the previous commit 85c2848 had failures but CI / test each commit (pull_request) passed

   not sure this is a followup someone can look at to fix?

8. 
   Prabhat1308 commented at 8:28 pm on July 6, 2025: contributor

   So one thing I noticed is that the tests passed, but the previous commit 85c2848 had failures but CI / test each commit (pull_request) passed

   not sure this is a followup someone can look at to fix?

   I could replicate this when I built cmake with

   0cmake -B build -DCMAKE_BUILD_TYPE=Debug
   

   This is because the specific piece of code is not enabled until there is the DEBUG_LOCKORDER flag.

   From the failures i noticed that failing tests had either -DCMAKE_BUILD_TYPE=Debug or "-DAPPEND_CPPFLAGS=-DDEBUG_LOCKORDER" in the config which is missing in ci-test-each-commit-exec.py that tests for each commit .

   adding any of the options should fix it . I guess "-DAPPEND_CPPFLAGS=-DDEBUG_LOCKORDER" is the better option ?

9. 
   maflcko commented at 7:45 am on July 7, 2025: member

   If we want more debug in the ci task, we could go full Debug type, but enable optimizations:

   0-DAPPEND_CXXFLAGS='-O3 -g2' -DAPPEND_CFLAGS='-O3 -g2' -DCMAKE_BUILD_TYPE=Debug
   

10. 
    maflcko commented at 11:14 am on July 7, 2025: member

    Done in https://github.com/bitcoin/bitcoin/pull/32888
11. 
    kevkevinpal commented at 2:33 pm on July 7, 2025: contributor

    Done in #32888

    awesome going to squash 41f64bd and 85c2848

12. threading: use correct mutex name in reverse_lock fatal error messages

    Now that REVERSE_LOCK requires the name of the actual mutex, it can be used for
    better error messages.

    de4eef52d1
13. theuni approved
14. 
    theuni commented at 2:34 pm on July 7, 2025: member

    ACK 41f64bd636f580851ef9aa27d56dc45885ace0c7.

    Thanks. I had the same test fixup locally on my branch, just forgot to push it.

15. kevkevinpal force-pushed on Jul 7, 2025
16. theuni approved
17. 
    theuni commented at 2:36 pm on July 7, 2025: member

    Re-ACK de4eef52d123b781b833841a9765d1788010ac6b
18. TheCharlatan approved
19. 
    TheCharlatan commented at 2:40 pm on July 7, 2025: contributor

    ACK de4eef52d123b781b833841a9765d1788010ac6b
20. fanquake merged this on Jul 7, 2025
21. fanquake closed this on Jul 7, 2025

    ---

Contributors
kevkevinpal DrahtBot Prabhat1308 maflcko theuni TheCharlatan