Skip to content

Fix HandleTableBucket::Contains to account for slots in DATAS#125569

Open
hoyosjs wants to merge 3 commits intodotnet:mainfrom
hoyosjs:juhoyosa/remove-dead-code
Open

Fix HandleTableBucket::Contains to account for slots in DATAS#125569
hoyosjs wants to merge 3 commits intodotnet:mainfrom
hoyosjs:juhoyosa/remove-dead-code

Conversation

@hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Mar 15, 2026

This fixes iteration of the handle table for a given bucket. This bug had hit the DAC iteration as well as this code path. Each bucket has a handle table that has as many slots as cores for server GC. This was 1:1 prior to DATAs by default - it also hd a bug when an explicit number of heaps was set. The only reason this hadn't had any issues prior to this is this function is unused.

The other option is to use the first commit in this PR - rev the major of the GC interface and delete the function altogether.

Copilot AI review requested due to automatic review settings March 15, 2026 01:00
@dotnet-policy-service
Copy link
Contributor

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

Copy link
Contributor

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 handle table bucket membership checks in CoreCLR GC by ensuring HandleTableBucket::Contains iterates over the actual number of handle-table slots allocated per bucket (which can differ from IGCHeap::GetNumberOfHeaps() in some configurations, e.g., with DATAs / explicit heap counts).

Changes:

  • Update HandleTableBucket::Contains to iterate using getNumberOfSlots() rather than g_theGCHeap->GetNumberOfHeaps().
  • Store the slot count in a local n_slots to match surrounding patterns and avoid repeated queries.

You can also share your feedback on Copilot code review. Take the survey.

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.

2 participants