Skip to content
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

[release/7.0] Issue was that if a BGC thread handles a mark stack overflow, #74814

Merged
merged 1 commit into from
Aug 31, 2022

Conversation

github-actions[bot]
Copy link
Contributor

Backport of #74571 to release/7.0

/cc @PeterSolMS

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

…s into yet another mark stack overflow on another heap, we set a flag on the region, and the containing heap. However, the BGC handling the other heap may have already decided to move on, and may thus not see the flag.

Fix is to set the flag on the heap doing the scan rather than the heap containing the object causing the mark stack stack overflow. The thread handling that heap will indeed recheck the flag and rescan if necessary. This necessitates another change because in the concurrent case, we need each BGC thread to enter mark stack overflow scanning if there was a mark stack overflow on its heap. So we need to propagate the per-heap flag to all the heaps.

Fixed another issue for regions where the small_object_segments local variable in background_process_mark_overflow_internal would be set incorrectly in the non-concurrent case. It would be set to FALSE as soon as all the regions for gen 0 are processed.
@ghost
Copy link

ghost commented Aug 30, 2022

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

Issue Details

Backport of #74571 to release/7.0

/cc @PeterSolMS

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-GC-coreclr

Milestone: -

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

approved. please get a code review and once we get a green ci we can merge.

@carlossanlop
Copy link
Member

Got an approval and a sign-off. The CI failure was a cancelled operation due to a timeout.
Ready to merge. :shipit:

@carlossanlop
Copy link
Member

Oh, the tests were re-executed. I'll have to wait.

@jeffschwMSFT
Copy link
Member

Sorry @carlossanlop :(. I missed your comment and was doing a sweep.

@carlossanlop
Copy link
Member

CI re-run shows a cancellation, but no test failures.
Approved and signed off.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit d1a7184 into release/7.0 Aug 31, 2022
@carlossanlop carlossanlop deleted the backport/pr-74571-to-release/7.0 branch August 31, 2022 02:04
@ghost ghost locked as resolved and limited conversation to collaborators Sep 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants