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

Disable mark list optimization if we hit a per region mark list overflow #86508

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

cshung
Copy link
Member

@cshung cshung commented May 19, 2023

Fixes microsoft/FASTER#835

In case the mark list is overflowed when we get_region_mark_list, we have to disable the optimization, otherwise, we will miss marking live objects and cause heap corruption.

@cshung cshung self-assigned this May 19, 2023
@ghost
Copy link

ghost commented May 19, 2023

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

Issue Details

Fixes microsoft/FASTER#835

In case the mark list is overflowed when we get_region_mark_list, we have to disable the optimization, otherwise, we will miss marking live objects and cause heap corruption.

Author: cshung
Assignees: cshung
Labels:

area-GC-coreclr

Milestone: -

Copy link
Member

@Maoni0 Maoni0 left a comment

Choose a reason for hiding this comment

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

fix looks fine but please consider not duplicating code in plan_phase.

@ghost ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 23, 2023
Copy link
Contributor

@PeterSolMS PeterSolMS left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Your solution has the property that once we hit a mark list overflow for one region, we will not use the mark list for any other region, even if it is possible. Given that this is such a rare case anyway, it doesn't seem worthwhile fixing.

@cshung cshung force-pushed the public/mark-list-overflow branch from 2812a50 to e23f73d Compare May 26, 2023 17:06
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label May 26, 2023
@cshung cshung requested a review from Maoni0 May 26, 2023 17:07
@Maoni0
Copy link
Member

Maoni0 commented May 26, 2023

this actually compiles? use_mark_list is not defined in this method or as a per heap/global member.

@cshung cshung force-pushed the public/mark-list-overflow branch from e23f73d to c82a4fe Compare May 27, 2023 02:40
Copy link
Member

@Maoni0 Maoni0 left a comment

Choose a reason for hiding this comment

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

LGTM!

@cshung cshung merged commit 1b0eb4d into dotnet:main Jun 5, 2023
@cshung cshung deleted the public/mark-list-overflow branch June 5, 2023 18:46
cshung added a commit to cshung/runtime that referenced this pull request Jun 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 5, 2023
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.

Random crashes when server concurrent GC enabled (didn't fixed)
3 participants