Fix DLL prop logic in Microsoft.Diagnostics.Tracing.TraceEvent.props and TraceEvent.csproj

[vancem]

In preparing PR #813 for issue #755 I have found that logic for dealing with DLL deployment is fragile and ad-hoc. This issue tracks cleaning this up in a better way.

Here is what I have learned.

We have 5 DLLs associated with the TraceEventPackage.

Microsoft.Diagnostics.FastSerialization.dll
Microsoft.Diagnostics.Tracing.TraceEvent.dll
OSExtensions.dll
TraceReloggerLib.dll
Dia2Lib.dll

As well as two native DLLs. KernelTraceControl.dll and msdia140.dll (which are architecture specific). This package preserves the illusion that the package is architecture independent by deploying all architectures (well x86, and amd64) and loading the appropriate one at runtime.

However currently two issues are being worked round in TraceEvent.csproj (when building the TraceEvent Dlls locally and Microsoft.Diagnostics.Tracing.TraceEvent.props (when unpacking the nuget package). They are

• The Dia2Lib and TraceReloggerLib are COM interop DLLs. C# has very nice support that takes the meta-data in such DLLs and embeds it into the DLL that referenced it (in this case Microsoft.Diagnostics.Tracing.TraceEvent.dll). On the command line the -link qualifier when using the -reference qualifier tells the C# to do this embedding. I don't know how to do this in MSBUILD (presumably it is a property on the References ItemList). When this is done you don't need the DiaLib and TraceReloggerLib at runtime. Currently for the msbuild DESKTOP runtime that copies references to the output directory ASSUME that these DLLs have been embedded but currently they are NOT, which causes a missing DLL at runtime error. Currently we solve this by copying these DLLs to the output directory, but it would be better if we embed them. However these DLLs are being referenced as a NUGET library (the microsoft.diagnostics.tracing.traceevent.supportfiles nuget package) and it is unclear how to make these references indicate they that need to be emedded. This is the first issue.
• The second issue is that both Microsoft.Diagnostics.Tracing.TraceEvent.dll and OSExtensions.dll load the native dlls by looking for them relative to the DLL location. In .NET Core (but not Desktop) these DLLs are loaded from the NUGET package caches, and the native DLLs are NOT in the correct relative location. Currently this works because
  1. The *.TraceEvent dll has a fallback probe location where that works for Nuget locations (hack)
  2. The OSExtentions.dll is explicitly copied to the target location. However we copy a NetStandard1.6 or Net45. This works only because this DLL's dependencies are simple and don't change (thus using one built for old frameworks works for new ones. It too is a hack.

This issue is to fix this. Currently the logic is very subtle (which is why I am documenting it here).

Running DLLs from the Nuget cache is a questionable thing in general and in V3.0 we seem to be moving away from it.

The ideal situation (in my opinion) is to get embedding to work (fix issue 1) and put in logic that copies the DLLs to the target location for old 1.* and 2.* runtimes and it just works for V3.0

[adamsitnik]

I think that @eerhardt knows how to solve this problem or knows the person that knows. But he is OOF until 2nd of January.

[vancem]

I am in no super-hurry, we can wait until Jan.

[suprak]

Maybe the below will already be covered by the work outlined here, but in case not,

There's an additional problem that exists when PerfView and related TraceEvent binaries are co-located in the same directory.

The scenario is that I own an app, that relies on both PerfView and TraceEvent so when my app deploys, those deploy with it.

PerfView has custom AppDomain.AssemblyResolve logic in UnpackResourcesIfNeeded that only remaps to unpacked version of binaries when an assembly resolution fails normal lookup.

This means that if PerfView and TraceEvent (+ OSExtensions, FastSerialization, etc) are in the same folder, PerfView may have some of it's dependencies auto resolved to what is along side it and not rewritten to the unpacked location.

[vancem]

@suprak, Your scenario is a known issue, but is not easy to fix because the NORMAL .NET lookup logic is what is kicking in. If you are using a compatible TraceEvent package (should almost always be the case, since PerfView mostly does not care), then there really is not a problem (PerfView may not be using the one it unpacked, but it is using one that will work). I don't know of a fix for this. (In particular the changes we are envisioning above will not fix it).

[suprak]

@vancem Wouldn't just moving to use the AppDomain.AssemblyLoad event instead do the trick?
This will allow you to override the dependency lookup and not just the failed to resolve cases.

[vancem]

@suprak, There may be tricks that I don't know, but AssemblyLoad is called AFTER an assembly is loaded (thus it is too late). As such it does not have any mechanism for influencing the loading of things. It turns out that predictability (ability to determine which DLL is loaded without running the code), is an important characteristic in the runtime so I don't think you will find a easy of doing what you want.

[suprak]

@vancem You're right, I misread the documentation.

Maybe another option is instead of waiting for CLR to kickoff assembly resolution, we could load the dependencies using reflection from the unpack location?

I think our particular issue wasn't as obvious in the past because PerfView's version of the dependency used to be something like 0.0.0.0 so there was no collisions with publicly released version of TraceEvent etc.

[vancem]

Assembly loading is trickier than it seems because there different 'contexts' (sets of loaded assemblies), and loading with LoadFrom is different than loading with normal loads. While I don't rule out a solution here I am actually pretty pessimistic in the ability to OVERRRIDE loading from the local directory (which is what you want). Feel free to experiment, but keep in mind that this is a pretty rare issue, so I don't want to do major surgery or unnatural things to fix this.

[suprak]

I worked "around" this by snapping the TraceEvent version to the same one used by the version of PerfView we rely on. Hopefully this will help ensure maximum compatibility.

In the future, we may have to sandbox either PerfView or TraceEvent from each other, if we need to safely rev the versions.

[eerhardt]

Sorry - I totally missed this thread while I was out and just found it in my inbox.

Issue 1 above

but it would be better if we embed them. However these DLLs are being referenced as a NUGET library (the microsoft.diagnostics.tracing.traceevent.supportfiles nuget package) and it is unclear how to make these references indicate they that need to be emedded. This is the first issue.

The MSBuild way of getting COM PIA types embedded is to add a COMReference Item to your .csproj and set EmbedInteropTypes=True. However, this only works using .NET desktop Framework msbuild.exe. It doesn't currently work using dotnet build in the .NET Core SDK. dotnet/msbuild#3986 is tracking this issue.

Issue 2 above

put in logic that copies the DLLs to the target location for old 1.* and 2.* runtimes and it just works for V3.0

Agreed. That seems like a simple enough fix - add a CopyToOutputDirectory item to ensure they are copied to the output directory always.

Native assembly loading

There may be tricks that I don't know, but AssemblyLoad is called AFTER an assembly is loaded (thus it is too late). As such it does not have any mechanism for influencing the loading of things

On .NET Core you can write your own AssemblyLoadContext and override protected virtual System.IntPtr LoadUnmanagedDll(string unmanagedDllName). This should allow you to intercept the load and do whatever you need to do.

[eerhardt]

Some more info on Issue 1:

I learned that NuGet added a feature for this. See https://github.com/NuGet/Home/wiki/Embed-Interop-Types-with-PackageReference. And the corresponding issue to uptake this new feature in the SDK: dotnet/sdk#2732.