Skip to content

Fix an intermittent cache unit test deadlock at exit - #13535

Merged
bneradt merged 1 commit into
apache:masterfrom
bneradt:fix-cache-unit-test-exit-deadlock
Aug 11, 2026
Merged

Fix an intermittent cache unit test deadlock at exit#13535
bneradt merged 1 commit into
apache:masterfrom
bneradt:fix-cache-unit-test-exit-deadlock

Conversation

@bneradt

@bneradt bneradt commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

The cache unit test harness starts the event and net processors but
never stops them, so every test binary reaches exit() with ET_NET
threads still running. Static destruction then frees globals out from
under those threads: the records table in RecCore.cc is destroyed while
a still-initializing event thread reads it through RecGetRecordInt(),
and the ts::Metrics storage blob is released while NetHandler's activity
loop increments a counter into it. Both are heap-use-after-frees, and
under ASan the reporting thread races the exiting main thread. Usually
the process dies first and the report is truncated to two lines with a
zero exit status, so ctest reports a pass; occasionally the report
deadlocks instead and the test hangs until ctest times it out after
1500 seconds. The short tests that never touch the cache lose this race
most often, which is why CacheAggregateWriteBuffer and CacheStripe are
the ones that fail.

This addresses the deadlock at its source by giving the harness's Catch2
listener a testRunEnded hook that shuts the event system down and joins
the event threads before the test binary returns from main. Once the
threads are gone, static destruction has no concurrent reader to race,
so neither use-after-free can be reported and the ASan reporting
deadlock cannot arise.

Verification

Built with --preset ci-rocky (ASan, Debug) and looped the affected
binaries directly, since a full ctest pass rarely trips the race and the
failing runs still exit zero:

Test Before After
CacheAggregateWriteBuffer 15 ASan reports / 4000 runs 0 / 2000 runs
CacheStripe 3 ASan reports / 600 runs 0 / 600 runs

The 15 reports split into the two use-after-frees described above: six
READ of size 8 in g_records_ht.find(), reached from
initialize_thread_for_net() on a thread that has not finished starting,
and nine WRITE of size 8 in ts::Metrics::Counter::increment() from
NetHandler::waitForActivity(). Both are freed by the main thread from
static destruction.

All 20 test_cache* plus test_ConfigVolumes targets pass with the fix, so
joining the event threads does not hang the tests that drive the cache and
already call TSSystemState::shut_down_event_system() themselves.

Ten full ctest -j4 passes were clean, which is expected: this bug reports
a zero exit status in the overwhelming majority of cases and is only visible
in CI when the ASan report deadlocks instead, as in the Rocky Linux build
that prompted this patch.

The cache unit test harness starts the event and net processors but
never stops them, so every test binary reaches exit() with ET_NET
threads still running. Static destruction then frees globals out from
under those threads: the records table in RecCore.cc is destroyed while
a still-initializing event thread reads it through RecGetRecordInt(),
and the ts::Metrics storage blob is released while NetHandler's activity
loop increments a counter into it. Both are heap-use-after-frees, and
under ASan the reporting thread races the exiting main thread. Usually
the process dies first and the report is truncated to two lines with a
zero exit status, so ctest reports a pass; occasionally the report
deadlocks instead and the test hangs until ctest times it out after
1500 seconds. The short tests that never touch the cache lose this race
most often, which is why CacheAggregateWriteBuffer and CacheStripe are
the ones that fail.

This addresses the deadlock at its source by giving the harness's Catch2
listener a testRunEnded hook that shuts the event system down and joins
the event threads before the test binary returns from main. Once the
threads are gone, static destruction has no concurrent reader to race,
so neither use-after-free can be reported and the ASan reporting
deadlock cannot arise.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 11, 2026 15:24
@bneradt bneradt added this to the 11.0.0 milestone Aug 11, 2026
@bneradt bneradt self-assigned this Aug 11, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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.

@bneradt bneradt added Cache Tests ASan Address Sanitizer labels Aug 11, 2026
@bneradt
bneradt merged commit 0c18d46 into apache:master Aug 11, 2026
15 checks passed
@bneradt
bneradt deleted the fix-cache-unit-test-exit-deadlock branch August 11, 2026 18:14
@github-project-automation github-project-automation Bot moved this to For v10.2.0 in ATS v10.2.x Aug 11, 2026
cmcfarlen pushed a commit that referenced this pull request Aug 12, 2026
The cache unit test harness starts the event and net processors but
never stops them, so every test binary reaches exit() with ET_NET
threads still running. Static destruction then frees globals out from
under those threads: the records table in RecCore.cc is destroyed while
a still-initializing event thread reads it through RecGetRecordInt(),
and the ts::Metrics storage blob is released while NetHandler's activity
loop increments a counter into it. Both are heap-use-after-frees, and
under ASan the reporting thread races the exiting main thread. Usually
the process dies first and the report is truncated to two lines with a
zero exit status, so ctest reports a pass; occasionally the report
deadlocks instead and the test hangs until ctest times it out after
1500 seconds. The short tests that never touch the cache lose this race
most often, which is why CacheAggregateWriteBuffer and CacheStripe are
the ones that fail.

This addresses the deadlock at its source by giving the harness's Catch2
listener a testRunEnded hook that shuts the event system down and joins
the event threads before the test binary returns from main. Once the
threads are gone, static destruction has no concurrent reader to race,
so neither use-after-free can be reported and the ASan reporting
deadlock cannot arise.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit 0c18d46)
@cmcfarlen cmcfarlen moved this from For v10.2.0 to Picked v10.2.0 in ATS v10.2.x Aug 12, 2026
@cmcfarlen cmcfarlen modified the milestones: 11.0.0, 10.2.0 Aug 12, 2026
@cmcfarlen

Copy link
Copy Markdown
Contributor

Cherry-picked to the 10.2.x branch as 51c0e97 for the 10.2.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ASan Address Sanitizer Cache Tests

Projects

Status: Picked v10.2.0

Development

Successfully merging this pull request may close these issues.

3 participants