Skip to content

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 8, 2021

Backport of #59989 to release/6.0

/cc @PeterSolMS

Customer Impact

This bug may cause random AVs and memory corruption. The root cause is a race condition between user threads allocating and GC thread 0 decommitting memory.

Testing

Ran GC stress test overnight with no issues.

Risk

Low. The bug crept in because of a change I made for regions that was too agressive. The proposed fix essentially goes back to the code before that change, which is what we shipped in 5.0.

We assume that we can use half the free list space in gen 0 for new allocation. If that is too optimistic, we may allocate into decommitted memory and crash in the allocator. That is because there is a race condition between the allocating thread and the decommitting thread - we decided to avoid that by making sure we would never decommit memory that we may allocate in gen 0.

There are two reasons why assuming we can use half the free list space for new allocations may be too optimistic:
 - if we allocate large objects in gen 0, we may not have free spaces of the necessary size available.
- when background GC goes into background_ephemeral_sweep, it deletes and rebuilds the free list for gen 0. A thread trying to allocate during that time may see a completely empty free list.
@ghost ghost added the area-GC-coreclr label Oct 8, 2021
@PeterSolMS PeterSolMS requested a review from Maoni0 October 11, 2021 13:28
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. We should take this for consideration in .NET 6.

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Oct 11, 2021
@jeffschwMSFT jeffschwMSFT added this to the 6.0.0 milestone Oct 11, 2021
@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Oct 11, 2021
@jeffschwMSFT jeffschwMSFT merged commit d262c21 into release/6.0 Oct 11, 2021
@akoeplinger akoeplinger deleted the backport/pr-59989-to-release/6.0 branch October 12, 2021 15:29
@ghost ghost locked as resolved and limited conversation to collaborators Nov 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclr Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants