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] Fix ALC lookup from the DAC #78420

Merged
merged 2 commits into from
Jan 4, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 15, 2022

Fixes dotnet/diagnostics#3565

Backport of #78400 to release/7.0

/cc @AaronRobinsonMSFT

Customer Impact

The dumpalc command in sos was broken in .NET 7 due to some refactoring. This impacts customers and runtime developers that are investigating crashing issues. Note that on non-Windows, the dumpalc command will crash the debugger without this fix. On Windows an error message is reported indicating a failure to acquire the ALC.

This was not reported by a customer. However, it was found by me when investigating a customer reported issue. The fact that it crashes the debugger is enough justification for me since investigating DMPs is a big concern because there is no easy way, other than dumpalc, to determine the ALC an object resides in.

An additional consideration here is the new feature being added to sos, dotnet/diagnostics#3508, will now cause a crash on non-Windows .NET 7 for the very common dumpmt command.

Testing

Validated with sos extension locally. The runtime/diagnostics repo has testing that is being added in dotnet/diagnostics#3508.

Risk

Minimal. This permits calling sos commands and has no product impact.

It seems the SOS commands for ALC lookup have
been broken for a bit. Some parts of the impl
weren't DAC-ized and caused failures.
@ghost
Copy link

ghost commented Nov 15, 2022

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #78400 to release/7.0

/cc @AaronRobinsonMSFT

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-AssemblyLoader-coreclr

Milestone: -

@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 7.0.x milestone Nov 15, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT added the Servicing-consider Issue for next servicing release review label Nov 15, 2022
@AaronRobinsonMSFT
Copy link
Member

/cc @tommcdon

@carlossanlop
Copy link
Member

carlossanlop commented Nov 15, 2022

@AaronRobinsonMSFT when this is ready, please get an approval from @jeffschwMSFT. If approved by Jeff, then please send an email to Tactics to request merge approval.

Do we want this in 7.0.1? I ask because the deadline for merging servicing fixes for that version was yesterday, but maybe we still have some runway if we hurry.

Edit: I just asked, we don't have runway. This will have to go into the January release.

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 7.0.x

@carlossanlop
Copy link
Member

@AaronRobinsonMSFT have you sent the Tactics email requesting approval? I can't find it in my inbox.

I ask because for the January release, we will only have a one day window to merge servicing PRs, so I want to make sure all the PRs are 100% ready on that day for me to just click the merge button. That window is from Nov. 29th to Nov. 30th.

The only thing left for this PR is to get Tactics approval.

@AaronRobinsonMSFT
Copy link
Member

@carlossanlop Sent.

@jeffschwMSFT jeffschwMSFT added the Servicing-approved Approved for servicing release label Nov 22, 2022
@jeffschwMSFT jeffschwMSFT modified the milestones: 7.0.x, 7.0.2 Nov 22, 2022
@jeffschwMSFT jeffschwMSFT removed the Servicing-consider Issue for next servicing release review label Nov 22, 2022
@jeffschwMSFT jeffschwMSFT modified the milestones: 7.0.2, 7.0.3 Nov 29, 2022
@hoyosjs hoyosjs linked an issue Dec 27, 2022 that may be closed by this pull request
@carlossanlop
Copy link
Member

Approved by Tactics for 7.0.3.
CI is green.
Signed off by area owners.
No OOB changes needed.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 5992707 into release/7.0 Jan 4, 2023
@carlossanlop carlossanlop deleted the backport/pr-78400-to-release/7.0 branch January 4, 2023 20:22
@ghost ghost locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dumpalc makes dotnet dump analyze exit
4 participants