Fix ThreadSanitizer data race warnings in unittest framework #4727
+72
−15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add mutex parameter to run_test_case() to protect shared test case fields - Protect tc->runner write with mutex in multi-threaded mode - Protect tc->result read/write synchronization with mutex - Add mutex protection for parallel_msg global variable in unittest_test - Basic runner (single-threaded) passes NULL for mutex parameter - Text runner (multi-threaded) passes its mutex to protect concurrent access Co-authored-by: sauwming <[email protected]>
- Clarify memory barrier semantics in tc->result synchronization comment - Make mutex initialization/cleanup more explicit in unittest_test.c - Ensure parallel_msg_mutex is NULL before creation and on error path Co-authored-by: sauwming <[email protected]>
- Remove redundant NULL assignment (static variables are auto-initialized) - Eliminate code duplication by consolidating mutex lock/unlock logic - Maintain same functionality with cleaner code structure Co-authored-by: sauwming <[email protected]>
Copilot
AI
changed the title
[WIP] Fix data race warnings reported by TSan
Fix ThreadSanitizer data race warnings in unittest framework
Jan 7, 2026
Member
|
Move tc->result assignment inside mutex-protected critical section. The test function is still called outside the mutex to avoid serialization, but the result is stored in a local variable first, then written to tc->result inside the mutex to prevent race with concurrent reads in get_first_running(). Co-authored-by: sauwming <[email protected]>
Contributor
Author
... Fixed in commit bf6d1d8. The issue was that |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
ThreadSanitizer reported data races in the unit test framework on three shared fields accessed by multiple threads without synchronization:
tc->runner,tc->result, andparallel_msg.Changes:
unittest.c: Added optional
pj_mutex_t *mutexparameter torun_test_case()tc->runnerwrites with mutex when running multi-threadedtc->resultwrites by storing result in local variable first, then writing inside mutex-protected critical sectionunittest_test.c: Added
parallel_msg_mutexto serialize concurrent writes to shared bufferpj_ansi_strxcat()calls inparallel_func()unittest_parallel_test()Implementation:
Motivation and Context
TSan warnings indicate undefined behavior from concurrent access to shared memory. These races could cause test framework instability or incorrect test results under high concurrency.
How Has This Been Tested?
make -j3- no compilation errorspjlib-test-x86_64-pc-linux-gnu- 20/21 tests pass (SSL timeout expected in sandbox)tc->resultwrite inside mutexTypes of changes
Checklist:
Original prompt
This section details on the original issue you should resolve
<issue_title>Attempt to fix data race warnings reported by TSan</issue_title>
<issue_description>### Describe the bug