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

[3.1 port] Fix debugger crash during unload of assemblies in ALC #28023

Merged
merged 1 commit into from Mar 25, 2020

Conversation

@hoyosjs
Copy link
Member

hoyosjs commented Feb 21, 2020

Issue dotnet/runtime#2317 reports that trying to use unloadable ALCs under the debugger often ends up in a crash. There was a missing implementation when removing values from the module cache which in turn triggered a sporadic bad error in the DAC

This has already been fixed in .NET 5 (see dotnet/runtime#32311). This PR ports that fix down to release/3.1.

Fixes dotnet/runtime#2317

Customer Impact

Inner loop get's heavily impacted for plugin scenarios. From dotnet/runtime#2317

Developing/using the use cases that where described in the .net core announcement under Assembly Unloadability is slowed down as it is not possible to debug any scenario that requires assembly unloading. Without this fix the application must be restarted or tested without a debugger attached. This makes plugin development very annoying and less attractive.

Also see https://developercommunity.visualstudio.com/content/problem/698374/vs2019-and-fatal-error-has-occurred-and-debugging.html

Regression?

Not a regression. Unloadable ALC's were introduced in the 3.0/3.1 time-frame as a new feature. However, adoption is cumbersome on inner loop due to this.

Testing

There's currently work in flight to add ALCs and unloadability verification for debugger scenarios.

Risk

Low. The only code path that reaches this is exactly the one that's getting fixed and that still has somewhat low adoption.

Code Reviewer

@sdmaclea

@hoyosjs hoyosjs added this to the 3.1.x milestone Feb 21, 2020
@hoyosjs hoyosjs requested a review from sdmaclea Feb 21, 2020
@hoyosjs hoyosjs self-assigned this Feb 21, 2020
@hoyosjs hoyosjs added this to Needs Triage in .NET Core Diagnostics via automation Feb 21, 2020
@tommcdon tommcdon moved this from Needs Triage to In Progress in .NET Core Diagnostics Feb 21, 2020
@jeffschwMSFT

This comment has been minimized.

Copy link
Member

jeffschwMSFT commented Feb 21, 2020

Approved for consideration for the April release.

@Anipik
Anipik approved these changes Feb 26, 2020
Copy link
Member

Anipik left a comment

LGTM, Area owner should also sign off

@Anipik Anipik removed the * NO MERGE * label Mar 25, 2020
@Anipik Anipik merged commit 46e980e into dotnet:release/3.1 Mar 25, 2020
35 checks passed
35 checks passed
WIP Ready for review
Details
coreclr-ci Build #20200221.1 succeeded
Details
coreclr-ci (Linux arm checked) Linux arm checked succeeded
Details
coreclr-ci (Linux arm64 checked) Linux arm64 checked succeeded
Details
coreclr-ci (Linux arm64 release) Linux arm64 release succeeded
Details
coreclr-ci (Linux x64 checked) Linux x64 checked succeeded
Details
coreclr-ci (Linux_musl x64 checked) Linux_musl x64 checked succeeded
Details
coreclr-ci (Linux_musl x64 release) Linux_musl x64 release succeeded
Details
coreclr-ci (Linux_rhel6 x64 release) Linux_rhel6 x64 release succeeded
Details
coreclr-ci (OSX x64 checked) OSX x64 checked succeeded
Details
coreclr-ci (Test Pri0 CoreFX Linux x64 checked) Test Pri0 CoreFX Linux x64 checked succeeded
Details
coreclr-ci (Test Pri0 CoreFX Windows_NT x64 checked) Test Pri0 CoreFX Windows_NT x64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux arm checked) Test Pri0 Linux arm checked succeeded
Details
coreclr-ci (Test Pri0 Linux arm64 checked) Test Pri0 Linux arm64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux x64 checked) Test Pri0 Linux x64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux_musl x64 checked) Test Pri0 Linux_musl x64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux_musl x64 release) Test Pri0 Linux_musl x64 release succeeded
Details
coreclr-ci (Test Pri0 OSX x64 checked) Test Pri0 OSX x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R Linux x64 checked) Test Pri0 R2R Linux x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R OSX x64 checked) Test Pri0 R2R OSX x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R Windows_NT x64 checked) Test Pri0 R2R Windows_NT x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R Windows_NT x86 checked) Test Pri0 R2R Windows_NT x86 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT arm64 checked) Test Pri0 Windows_NT arm64 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT x64 checked) Test Pri0 Windows_NT x64 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT x86 checked) Test Pri0 Windows_NT x86 checked succeeded
Details
coreclr-ci (Windows_NT arm checked) Windows_NT arm checked succeeded
Details
coreclr-ci (Windows_NT arm release) Windows_NT arm release succeeded
Details
coreclr-ci (Windows_NT arm64 checked) Windows_NT arm64 checked succeeded
Details
coreclr-ci (Windows_NT arm64 release) Windows_NT arm64 release succeeded
Details
coreclr-ci (Windows_NT x64 checked) Windows_NT x64 checked succeeded
Details
coreclr-ci (Windows_NT x64 debug) Windows_NT x64 debug succeeded
Details
coreclr-ci (Windows_NT x64 release) Windows_NT x64 release succeeded
Details
coreclr-ci (Windows_NT x86 checked) Windows_NT x86 checked succeeded
Details
coreclr-ci (Windows_NT x86 debug) Windows_NT x86 debug succeeded
Details
license/cla All CLA requirements met.
Details
.NET Core Diagnostics automation moved this from In Progress to Done Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.