ICU-20570 Fix data race in initializeFCD via UInitOnce#4086
Open
sfc-gh-bhannel wants to merge 2 commits into
Open
ICU-20570 Fix data race in initializeFCD via UInitOnce#4086sfc-gh-bhannel wants to merge 2 commits into
sfc-gh-bhannel wants to merge 2 commits into
Conversation
|
|
sfc-gh-bhannel
force-pushed
the
bhannel-fix-initializeFCD-data-race
branch
from
July 20, 2026 21:02
344df34 to
3b22d4e
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
Add TestInitializeFCD to utility/MultithreadTest — the suite run under TSAN in ICU's upstream CI. The test spawns 16 threads that concurrently construct StringSearch objects, preceded by u_cleanup() to reset g_nfcImpl. A spin barrier (u_atomic_int32_t gate) ensures all threads converge on initializeFCD simultaneously for reliable race detection. On unfixed builds, TSAN reports a data race at usearch.cpp:93 (concurrent write/read of g_nfcImpl). With the UInitOnce fix the test passes cleanly (exit 0, no TSAN warnings). tsan_test_results.txt records the full before/after commands and output.
Replace the racy null-check in initializeFCD() with umtx_initOnce so
g_nfcImpl is initialized exactly once across concurrent callers:
- Add static UInitOnce gSearchNFCInitOnce (using {} for ICU 79 compat;
U_INITONCE_INITIALIZER was removed after ICU 61)
- Extract initNFCImpl() as the once-init callback
- Call umtx_initOnce(gSearchNFCInitOnce, &initNFCImpl, status) in place
of the racy if (g_nfcImpl == nullptr) block
- Reset gSearchNFCInitOnce in usearch_cleanup() so u_cleanup() works
sfc-gh-bhannel
force-pushed
the
bhannel-fix-initializeFCD-data-race
branch
from
July 20, 2026 21:06
3b22d4e to
2d4d972
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
sfc-gh-bhannel
marked this pull request as ready for review
July 20, 2026 21:06
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
If multiple threads try to use ICU for the first time in the process concurrently, ICU initialization will have a data race inside of
initializeFCD.UInitOnceseems to be the standard pattern used widely in ICU, so I am applying it here as well. Here is a standalone repro of the issue: https://godbolt.org/z/9PPK84KT7Reproduction and Testing
I added a new unit test which reproduces the issue under TSAN:Checklist