ICU-23352 Reset initOnce before clearing the collation root singleton by ManoharPaturi · Pull Request #4259 · unicode-org/icu · GitHub
Skip to content

ICU-23352 Reset initOnce before clearing the collation root singleton - #4259

Open
ManoharPaturi wants to merge 1 commit into
unicode-org:mainfrom
ManoharPaturi:icu-23352-collation-root-cleanup-order
Open

ManoharPaturi wants to merge 1 commit into
unicode-org:mainfrom
ManoharPaturi:icu-23352-collation-root-cleanup-order

Conversation

@ManoharPaturi

@ManoharPaturi ManoharPaturi commented Oct 4, 2026 •

Copy link
Copy Markdown

Description

uprv_collation_root_cleanup() cleared rootSingleton before resetting initOnce. A concurrent CollationRoot::getRoot() that had already passed umtx_initOnce() could then dereference the freed singleton during library teardown. This change resets initOnce first, so concurrent callers are directed through initialization instead of reaching for the cleared pointer, as prescribed in the ticket.

The analysis and the fix direction come from ticket ICU-23352; an earlier pull request #3921 proposed the same change; the ticket was moved to Accepted while that PR was open, and after the discussion of the u_cleanup() threading contract there the PR went quiet and was closed without a technical rejection.

The regression test TestCollationRootCleanupRace (ported from #3921) is included in tsmthred.cpp/tsmthred.h: it runs ten rounds of u_cleanup() followed by concurrent CollationRoot::getRoot() reader threads, failing under TSAN/ASAN on unfixed code.

Testing

The race window is not deterministically observable in a regular test run, so this change ships without a new test. The ordering change is two lines, and the full rbbi and collation suites pass locally with it applied.

Copilot AI balanced review requested due to automatic review settings October 4, 2026 04:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@markusicu

markusicu commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

uprv_collation_root_cleanup() cleared rootSingleton before resetting
initOnce. A concurrent CollationRoot::getRoot() that had already passed
umtx_initOnce() could then dereference the freed singleton. Reset
initOnce first so concurrent callers are directed through
initialization instead of the cleared pointer.
@ManoharPaturi
ManoharPaturi force-pushed the icu-23352-collation-root-cleanup-order branch from 1d65f4c to 2460e8b Compare October 8, 2026 16:57
@jira-pull-request-webhook

Copy link
Copy Markdown

Notice: the branch changed across the force-push!

  • icu4c/source/test/intltest/tsmthred.cpp is now changed in the branch
  • icu4c/source/test/intltest/tsmthred.h is now changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@ManoharPaturi

ManoharPaturi commented Oct 8, 2026 •

Copy link
Copy Markdown
Author

This branch has not been deployed

No deployments
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.

3 participants