dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.3k stars 4.74k forks source link

Fix for https://github.com/dotnet/coreclr/issues/19762 has been lost in 2.1.x and 3.0.x #13537

Closed chrisnas closed 4 years ago

chrisnas commented 5 years ago

It seems that https://github.com/dotnet/coreclr/pull/19761 has been "lost" in 2.1.x, 2.2.x and 3.0.x. A new PR for "3.0 master" is on its way and it should be back-ported or no managed type name will be visible (UNKNOWN) in the dumps generated by createdump.exe

noahfalk commented 5 years ago

Thanks @chrisnas! Hopefully Mike can take a look at the PR whenever it is ready.

mikem8361 commented 5 years ago

You shouldn't need the createdump fix from dotnet/coreclr#19761 anymore with the new SOS/dotnet-dump support. If the metadata is missing from the dump (no matter what runtime it is created on), SOS will now find or downloaded the module and "map" the metadata into the address space.

Are you still seeing "UNKNOWN" types or stack function names with the latest SOS?

kevingosse commented 5 years ago

We're still seeing UNKNOWN with the latest SOS, yes. The memory dumps were taken with createdump, not dotnet-dump. I don't know if it makes a difference.

I'm guessing the issue is with the "download the module" part. We're taking memory dumps from production servers then open them on another machine that doesn't have access to the application binaries. Is that scenario still supported with SOS and createdump?

mikem8361 commented 5 years ago

I should have qualified my statement to the .NET Core runtime assemblies that we can download from the symbol server. Yes, dotnet-dump does use createdump to generate the core dump.

Yes, if the original assemblies are not available locally anywhere, you could still get the UNKNOWN type/function messages. The problem (and probably the reason this change never made it into 2.1 and it was removed from 3.0) with this temporary change is that it does add quite a bit to a core dump's size.

A possible workaround is to download your app assemblies to a local directory and add that directory to the symbol search path with setsymbolserver -directory <path> and then SOS should map those assemblies into the core dumps address space.

I need to look into this a little more before we add this createdump change back.

kevingosse commented 5 years ago

Back then we considered that the difference in size was negligible, but we deal with 10+ GB memory dumps, so we're definitely not representative of the "common" use-cases. Maybe we could just add another dump mode?

chrisnas commented 5 years ago

I've created a simple .NET Core console app and use createdump to generate a dump file. On the same machine, I'm using tooling based on ClrMD such as pstacks (https://github.com/chrisnas/DebuggingExtensions/releases/tag/1.6.2) and I still have errors on types. In addition, we don't have "vanilla" versions of Core CLR so it might be cumbersome to manage symbols also for the runtime

mikem8361 commented 5 years ago

You need this in 2.1 and 3.0? Would the master branch (5.0) be enough?

The --full option (or COMPlus_DbgMiniDumpType=4) on 3.0 (not 2.1) might be an option? The dumps are not that much bigger (in my limited testing) than a heap dump AND include the module memory. There was fix in 3.0 to "full" dumps in createdump so that the unreserved (freed) memory isn't included.

Would some kind of out of band "dotnet-dump" option to collect/package all the dump's assemblies/modules be an option? This is a very rough idea and we would probably also have to deal with the .NET Core SDK requirement the our global tools have.

sdmaclea commented 5 years ago

The --full option to createdump seems adequate to fix this issue.

It is unlikely we would backport this fix to 3.1, 3.0, or 2.1. If it can be fixed it will be fixed in out of band tooling from the dotnet/diagnostics repo.

I am closing this issue as will not fix. If a fix is not currently in the master branch we will not consider backporting it.

chrisnas commented 5 years ago

The --full option is not an option: the dump files are gigantic in 2.2 Why not taking my PR that is fixing the issue (even with a --withModule option for example? I think that createdump ships with .NET Core, not diagnostics repo in 2.2: no need to build/install a new tool such as dotnet-dump

sdmaclea commented 5 years ago

The --full option is not an option: the dump files are gigantic in 2.2

2.2 is end of life in a few weeks. We will not consider servicing it for this.

Why not taking my PR that is fixing the issue

We can discuss a master PR for .NET 5.0.

We generally do not add new features to older releases. Servicing fixes for released runtimes are tightly scrutinized.

no need to build/install a new tool such as dotnet-dump

We have a lot more flexibiliy to service these tools. I was thinking createdump might not be really tightly coupled to the runtime. If we were to move this tool out-of-band we would have a much easier time servicing the dump feature.