Skip to content

throw an exception instead of infinite loop in sort_mark_list #115492

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
May 13, 2025

Conversation

Maoni0
Copy link
Member

@Maoni0 Maoni0 commented May 12, 2025

we had a customer who observed an infinite loop in sort_mark_list due to heap corruption - the object that got marked was in a region that was already freed so region_limit is 0. we detect this and throw an exception which is easier for prod diag than having to deal with an infinite loop.

@Copilot Copilot AI review requested due to automatic review settings May 12, 2025 21:22
@Maoni0 Maoni0 changed the title throw an exception instead of infinite loop throw an exception instead of infinite loop in sort_mark_list May 12, 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 addresses a critical infinite loop issue observed in sort_mark_list due to heap corruption by introducing an exception mechanism when an invalid (zero) region limit is encountered.

  • Adds a check for a zero region limit and terminates using FATAL_GC_ERROR() instead of entering an infinite loop.
  • Updates the core garbage collection routine to improve production diagnostics when encountering freed regions.
Comments suppressed due to low confidence (1)

src/coreclr/gc/gc.cpp:10798

  • Consider verifying that FATAL_GC_ERROR() provides sufficient diagnostic context for production debugging, potentially by enhancing the error message or logging detailed information.
if (region_limit == 0)

Copy link
Contributor

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

@Maoni0 Maoni0 force-pushed the crash_instead_of_loop branch from f1d09c4 to 059365a Compare May 12, 2025 21:35
// simply throwing an exception.
if (region_limit == 0)
{
FATAL_GC_ERROR();
Copy link
Member

Choose a reason for hiding this comment

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

should we log something here to mention that its probably a heap corruption?

Copy link
Member Author

Choose a reason for hiding this comment

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

we chatted offline.

@Maoni0 Maoni0 merged commit be4dd91 into dotnet:main May 13, 2025
95 checks passed
@Maoni0
Copy link
Member Author

Maoni0 commented May 13, 2025

/backport to release/8.0-staging

@Maoni0
Copy link
Member Author

Maoni0 commented May 13, 2025

/backport to release/9.0-staging

Copy link
Contributor

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

Copy link
Contributor

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

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.

2 participants