Skip to content

fixing a bug in card mark stealing #117968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 24, 2025
Merged

fixing a bug in card mark stealing #117968

merged 1 commit into from
Jul 24, 2025

Conversation

Maoni0
Copy link
Member

@Maoni0 Maoni0 commented Jul 23, 2025

this fixes a problem with card mark stealing where we missed clamping the card clearing by the card stealing unit in card_transition. for this bug to appear the following conditions need to be met -

  • an object A straddles the 2mb card stealing unit and originally for that object a card below the 2mb boundary and a card that corresponds to at least 256 bytes above the 2mb boundary are set. and there are no reference fields inbetween.

  • one thread T0 is working on the 1st 2mb and discovers A and the first set card bit. this card doesn't need to be set, so poo is set the address that's described by the 2nd card since there're no reference fields inbetween. so card_transition is called which will call clear_cards on [1st card, (2nd card. and it stops at this line -
    card_table [end_word] &= highbits (~0, bits);
    where it sees end_card with the 2nd card still set, but before it writes it back to card_table[end_word]

  • meanwhile, another thread T1 needs to be working on the memory starting from this 2mb boundary. it discovers the 2nd card doesn't need to be set, and none of the cards that correspond to the card bundle bit needs to be set so it clears the cards and the card bundle bit.

  • now T0 writes back to card_table[end_word] with the 2nd card bit set.

it's not a problem when a card that shouldn't be set is set, given that its corresponding card bundle bit is also set. but it's definitely a problem if a card is set but its card bundle bit isn't, because next time when we have a cross gen reference, what's supposed to happen in the write barrier is either the card isn't already set and the WB will set the card and its corresponding card bundle bit, or the card is set and the WB wouldn't do anything. but now we have a situation where the card is set but the card bundle bit isn't, it just means the next GC that should be looking at this card wouldn't, if there were no other cards covered by that card bundle bit got newly set by the WB.

the cleanest fix is to make sure we don't step outside of the 2mb boundary when we call clear_cards in card_transition.

this issue was very hard to observe and debug - full credit goes to @ChrisAhna who also verified the fix.

@Copilot Copilot AI review requested due to automatic review settings July 23, 2025 06:06
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 23, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a race condition bug in the garbage collector's card mark stealing mechanism where multiple threads could incorrectly manage card table state across 2MB boundaries. The fix prevents one thread from clearing cards beyond its assigned 2MB card stealing unit, which could lead to inconsistent state between card bits and card bundle bits.

  • Introduces proper boundary checking when clearing cards in card_transition
  • Ensures card clearing operations are clamped to the card stealing unit limit
  • Prevents race conditions that could leave cards set without corresponding card bundle bits
Comments suppressed due to low confidence (2)

Copy link
Contributor

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

@Maoni0 Maoni0 enabled auto-merge (squash) July 23, 2025 21:41
@Maoni0
Copy link
Member Author

Maoni0 commented Jul 24, 2025

I also did some stress runs and didn't find any problems.

@Maoni0
Copy link
Member Author

Maoni0 commented Jul 24, 2025

/ba-g Known issue dotnet/dnceng#6004

@Maoni0 Maoni0 merged commit 587f703 into dotnet:main Jul 24, 2025
97 of 100 checks passed
@Maoni0
Copy link
Member Author

Maoni0 commented Jul 24, 2025

/backport to release/8.0-staging

Copy link
Contributor

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/16489375887

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

Successfully merging this pull request may close these issues.

3 participants