This repository has been archived by the owner on Apr 22, 2022. It is now read-only.
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
FileLoadException: Add Profiler fix (DataDog#671)
Fix FileLoadException: Loading this assembly would produce a different grant set from other instances. (Exception from HRESULT: 0x80131401) ## Problem ### Issue 1 Our CLR Profiler currently assumes that as long as the `Datadog.Trace.ClrProfiler.Managed` assembly has been loaded domain-neutral once, all AppDomains can use that assembly. Under this false assumption, we always instrument domain-neutral assemblies which adds an AssemblyReference to our managed profiler and affects how the CLR shares code between AppDomains. However, some applications may have BindingRedirect's specified on dependencies of the managed profiler like `System.Net.Http` which can force `Datadog.Trace.ClrProfiler.Managed` to no longer be loaded domain-neutral. When we instrument a domain-neutral assembly with a non-domain-neutral `Datadog.Trace.ClrProfiler.Managed` in one AppDomain and then another AppDomain tries to use the instrumented domain-neutral assembly, the second AppDomain crashes with the FileLoadException. ### Issue 2 Our new mechanism for automatically instrumenting AspNet uses the CLR Profiler to intercept a method call inside `System.Web` to call into `Datadog.Trace.ClrProfiler.Managed`. `System.Web` is always loaded domain-neutral but, as noted above, there are circumstances that can cause our assembly to not be loaded domain-neutral. We need to change the way we invoke the AspNet integration so that the assembly that runs the integration code is always domain-neutral. ## Changes proposed in this pull request: ### Addressing Issue 1 - Modify the profiler to never inject method calls to `Datadog.Trace.ClrProfiler.Managed` if the calling assembly was loaded domain-neutral. ### Addressing Issue 2 - Move basic types like `InterceptMethodAttribute` and friends to a new `Datadog.Trace.ClrProfiler.Managed.Core` that has no external dependencies. - Move the AspNet integration into a new assembly called `Datadog.Trace.AspNet.Loader` whose only dependencies are `Datadog.Trace.ClrProfiler.Managed.Core` and `System.Web`. This assembly closure guarantees that the assembly will always be loaded domain-neutral. This is now a safe entry point for `System.Web` to call into during application setup. Also, the method responsible for adding the AspNet integration does not wrap any methods (that would require `Sigil`). - To enable the minimal method call above that is simply called and returns to the caller without wrapping any target method, modify the method replacement infrastructure to support an `action`. The default value is `ReplaceTargetMethod` which performs the usual method replacement as before, but a new action `InsertFirst` will add a call to the method as the first instruction in the matching caller method. When generating `integrations.json`, add checks on the new `InsertFirst` integration methods. ## Consequences - Fix `Loading this assembly would produce a different grant set from other instances. (Exception from HRESULT: 0x80131401)` - For ASP.NET applications (or other scenarios where `LoaderOptimizationAttribute=MultiDomainHost`), there may be missing spans from calls between framework assemblies. Affected integrations include: - HttpMessageHandler - WebRequest - Wcf (server)
- Loading branch information
1 parent
389aa82
commit 7f78771