Skip to content

Fix bthread id butex state races - #3689

Open
songgaoye wants to merge 1 commit into
apache:masterfrom
songgaoye:fix/bthread-id-join-race
Open

songgaoye wants to merge 1 commit into
apache:masterfrom
songgaoye:fix/bthread-id-join-race

Conversation

@songgaoye

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: resolve

Problem Summary:

  • bthread_id accesses butex state words as plain uint32_t values even though butex manages them as atomic state. Concurrent bthread_id_join() and bthread_id_unlock_and_destroy() can therefore form a C++ data race.
    What is changed and the side effects?

Changed:

  • Store ID butex state as butil::atomic.
  • Centralize atomic version-word access helpers.
  • Add a concurrent join/destroy regression test.

Side effects:

  • Performance effects:

  • Breaking backward compatibility:


Check List:

@songgaoye
songgaoye force-pushed the fix/bthread-id-join-race branch from 8e93fe6 to 58f5f92 Compare October 28, 2026 15:58
@songgaoye

Copy link
Copy Markdown
Contributor Author

run tsan on master branch.

Running main() from /tmp/brpc-master-tsan/build-tsan/googletest-src/googletest/src/gtest_main.cc
Note: Google Test filter = BthreadIdTest.join_races_with_destroy
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from BthreadIdTest
[ RUN      ] BthreadIdTest.join_races_with_destroy
==================
WARNING: ThreadSanitizer: data race (pid=344302)
  Write of size 4 at 0x729400000040 by main thread:
    #0 bthread_id_unlock_and_destroy /tmp/brpc-master-tsan/src/bthread/id.cpp:630
    #1 TestBody /tmp/brpc-master-tsan/test/bthread_id_unittest.cpp:143
    #2 testing::Test::Run()
    #3 testing::TestInfo::Run()
    #4 testing::TestSuite::Run()
    #5 testing::internal::UnitTestImpl::RunAllTests()
    #6 testing::UnitTest::Run()
    #7 RUN_ALL_TESTS()
    #8 main

  Previous atomic read of size 4 at 0x729400000040 by thread T2:
    #0 std::__atomic_base<int>::load(std::memory_order)
    #1 bthread::butex_wait(void*, int, timespec const*, bool)
       /tmp/brpc-master-tsan/src/bthread/butex.cpp:755
    #2 bthread_id_join
       /tmp/brpc-master-tsan/src/bthread/id.cpp:530
    #3 join_race_waiter
       /tmp/brpc-master-tsan/test/bthread_id_unittest.cpp:125

  Thread T2 created by main thread at:
    #0 pthread_create
    #1 TestBody /tmp/brpc-master-tsan/test/bthread_id_unittest.cpp:137

SUMMARY: ThreadSanitizer: data race
  /tmp/brpc-master-tsan/src/bthread/id.cpp:630
  in bthread_id_unlock_and_destroy
==================
[       OK ] BthreadIdTest.join_races_with_destroy (260 ms)
[----------] 1 test from BthreadIdTest (260 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (260 ms total)
[  PASSED  ] 1 test.
ThreadSanitizer: reported 1 warnings
EXIT_CODE=66

@songgaoye
songgaoye force-pushed the fix/bthread-id-join-race branch from 58f5f92 to ed9a32f Compare October 28, 2026 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant