Skip to content

Take each block's zero lock in the incremental start-age test hook - #437

Open
dimensionscape wants to merge 1 commit into
mainfrom
fix/incremental-test-hook-reclaim-race
Open

dimensionscape wants to merge 1 commit into
mainfrom
fix/incremental-test-hook-reclaim-race

Conversation

@dimensionscape

Copy link
Copy Markdown
Member

Fixes #436.

__hxcpp_gc_incremental_test(10), which IncrementalGcStartSweep calls between collections, read every block's start bits and object headers without a lock. The zeroing thread that a collection starts keeps running while the program does. Under each block's mZeroLock it reclaims the blocks a finished incremental cycle left unswept and zeroes their holes, which writes the start bits (the memset in BlockDataInfo::reclaim<false>) and the rows the hook reads. Both TSan reports on main are that pair, with T1 the pool's first worker running ZeroAsyncJit. It needs the zeroing thread to be on a block while the hook reads it, so most runs passed.

The hook now holds each block's mZeroLock while it reads the block, as the incremental sweep (SweepBlocks) does, so it sees each block before or after the zeroing thread's work on it.

Nothing outside HXCPP_GC_INCREMENTAL_TEST reads blocks this way:

  • The allocator claims a block with a compare-and-swap on the same lock before it uses it (GetNextFree); the reads before that are the hints tsan.supp already describes.
  • A collection stops the zeroing thread before it reclaims, moves or cancels a cycle (StopThreadJobs, or StopZeroingLeft before weak processing and in every CancelIncremental path).
  • A slice that leaves the thread running first reclaims the free blocks under the lock. After that the thread only zeroes holes, whose start bits are clear, so conservative marking does not read them.

So programs could not hit this, and there is no Changes.md line.

Tests: the TSan job's run of IncrementalGcStartSweep (Check with TSan) is the check. It failed on main at 5a9b560 and c900b35; this PR's TSan job is rerun several times to show it passing.

__hxcpp_gc_incremental_test(10), which IncrementalGcStartSweep calls
between collections, read every block's start bits and object headers
without a lock. The zeroing thread that a collection starts keeps
running while the program does: under each block's mZeroLock it
reclaims the blocks a finished cycle left unswept and zeroes their
holes, which writes the start bits and rows the hook reads. TSan
reported the pair on main twice (#436). It needs the zeroing thread to
be on a block while the hook reads it, so most runs passed.

The hook now holds each block's mZeroLock while it reads the block, as
the incremental sweep (SweepBlocks) does.

Nothing outside HXCPP_GC_INCREMENTAL_TEST reads blocks this way. The
allocator claims a block through the same lock before it uses it. A
collection stops the zeroing thread before it reclaims, moves or
cancels a cycle. A slice that leaves it running first reclaims the free
blocks under the lock, after which the thread only zeroes holes, which
marking does not read. So programs could not hit this, and there is no
Changes.md line.

Fixes #436
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.

TSan: IncrementalGcStartSweep's test hook reads a block a GC worker is reclaiming

1 participant