Repository navigation
Take each block's zero lock in the incremental start-age test hook - #437
Open
dimensionscape wants to merge 1 commit into
Open
dimensionscape wants to merge 1 commit into
dimensionscape wants to merge 1 commit into
Conversation
__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
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
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.
Fixes #436.
__hxcpp_gc_incremental_test(10), whichIncrementalGcStartSweepcalls 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'smZeroLockit reclaims the blocks a finished incremental cycle left unswept and zeroes their holes, which writes the start bits (thememsetinBlockDataInfo::reclaim<false>) and the rows the hook reads. Both TSan reports on main are that pair, with T1 the pool's first worker runningZeroAsyncJit. 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
mZeroLockwhile 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_TESTreads blocks this way:GetNextFree); the reads before that are the hintstsan.suppalready describes.StopThreadJobs, orStopZeroingLeftbefore weak processing and in everyCancelIncrementalpath).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.