Skip to content

[release/8.0-staging] Fix for a stress crash in ComAwareWeakReferenceNative::HasInteropInfo #94649

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

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 12, 2023

Backport of #94643 to release/8.0-staging

/cc @VSadov

Customer Impact

Fixes: #94579

The issue was reported by an external customer. In rare cases when creating a weak reference instance an application may crash with ExecutionEngineException in the runtime while probing for COM interop info.

The repro is a fairly large WinForms app (paint.net)

When run with GCStress=2 the crash is nearly 100% certain.

Testing

Regular testing.
Manually validated that the customer repro no longer causes EE crashes. That includes running it under GCStress=2

Risk

Low. The fix is adding a missing null-check where we had an assert.
Turns out the asserted condition may not hold when racing with GC and must be handled.

@teo-tsirpanis teo-tsirpanis added this to the 8.0.x milestone Nov 13, 2023
@carlossanlop
Copy link
Member

Friendly reminder: If you'd like this to be included in the December release, please merge it before Tuesday November 14th EOD (Code Complete).

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 will take for consideration in 8.0.x

@AaronRobinsonMSFT AaronRobinsonMSFT added the Servicing-approved Approved for servicing release label Nov 14, 2023
@AaronRobinsonMSFT AaronRobinsonMSFT added Servicing-consider Issue for next servicing release review and removed Servicing-approved Approved for servicing release labels Nov 14, 2023
@carlossanlop
Copy link
Member

@AaronRobinsonMSFT @jeffschwMSFT did this end up getting Tactics approval? Today is code complete.

@jeffschwMSFT
Copy link
Member

Today is code complete.

@carlossanlop for January?

@carlossanlop
Copy link
Member

@carlossanlop for January?

No, it's code complete for December: https://dev.azure.com/devdiv/DevDiv/_wiki/wikis/DevDiv.wiki/38691/December-2023

@carlossanlop
Copy link
Member

I'm starting the flow from staging to internal now, so the branches are closed. This will have to wait until the January release.

@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 16, 2023
@leecow leecow modified the milestones: 8.0.x, 8.0.1 Nov 16, 2023
@jeffschwMSFT jeffschwMSFT merged commit 7dc2e67 into release/8.0-staging Nov 21, 2023
@jkotas jkotas deleted the backport/pr-94643-to-release/8.0-staging branch December 2, 2023 01:31
@github-actions github-actions bot locked and limited conversation to collaborators Jan 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants