diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/HttpModule_Integration.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/HttpModule_Integration.cs index 8a23ebc276c6..c9b7e92d111d 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/HttpModule_Integration.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/HttpModule_Integration.cs @@ -21,31 +21,25 @@ namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.AspNet TypeName = "System.Web.Compilation.BuildManager", MethodName = "InvokePreStartInitMethodsCore", ReturnTypeName = ClrNames.Void, - ParameterTypeNames = new[] { "System.Collections.Generic.ICollection`1[System.Reflection.MethodInfo]", "System.Func`1[System.IDisposable]" }, + ParameterTypeNames = ["System.Collections.Generic.ICollection`1[System.Reflection.MethodInfo]", "System.Func`1[System.IDisposable]"], MinimumVersion = "4.0.0", MaximumVersion = "4.*.*", - IntegrationName = IntegrationName)] + IntegrationName = nameof(IntegrationId.AspNet))] [Browsable(false)] [EditorBrowsable(EditorBrowsableState.Never)] public class HttpModule_Integration { - private const string IntegrationName = nameof(IntegrationId.AspNet); + // WARNING: Do not add a static reference to `IDatadogLogger` or reference + // anything related to Tracer.Instance etc. This method is called at application + // start _before_ the Tracer is initialized; adding additional references could + // cause recursion issues and deadlocks in some scenarios, e.g. where there are + // multiple apps per app pool. /// /// Indicates whether we're initializing the HttpModule for the first time /// private static int _firstInitialization = 1; - /// - /// OnMethodBegin callback - /// - /// Type of the target - /// Type of the collection - /// Type of the - /// Instance value, aka `this` of the instrumented method. This method is static so this parameter will always be null - /// The methods to be invoked - /// The function to set the environment culture - /// Calltarget state value internal static CallTargetState OnMethodBegin(TTarget instance, TCollection methods, TFunc setHostingEnvironmentCultures) { if (Interlocked.Exchange(ref _firstInitialization, 0) != 1) @@ -61,7 +55,8 @@ internal static CallTargetState OnMethodBegin(TTarg catch { // Unable to dynamically register module - // Not sure if we can technically log yet or not, so do nothing + // We can't log here as it could cause recursion issues and deadlocks in some scenarios + // where there are multiple apps per app pool } return CallTargetState.GetDefault(); diff --git a/tracer/src/Datadog.Trace/ClrProfiler/CallTarget/CallTargetInvoker.cs b/tracer/src/Datadog.Trace/ClrProfiler/CallTarget/CallTargetInvoker.cs index c57c40d206c5..8ee5955982cc 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/CallTarget/CallTargetInvoker.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/CallTarget/CallTargetInvoker.cs @@ -6,7 +6,9 @@ using System; using System.ComponentModel; +using System.Diagnostics; using System.Runtime.CompilerServices; +using Datadog.Trace.ClrProfiler.AutoInstrumentation.AspNet; using Datadog.Trace.ClrProfiler.CallTarget.Handlers; namespace Datadog.Trace.ClrProfiler.CallTarget; @@ -18,14 +20,67 @@ namespace Datadog.Trace.ClrProfiler.CallTarget; [EditorBrowsable(EditorBrowsableState.Never)] public static class CallTargetInvoker { + // NOTE: Do not add a reference to IDatadogLogger or Tracer.Instance etc here. + // On .NET FX, this class may be called _before_ Instrumentation.Initialize, if + // there are multiple apps running in the same app pool. Referencing IDatadogLogger + // can cause some things (e.g. AppSettings) to initialized too early, which can + // cause recursion issues and deadlocks. + +#if NETFRAMEWORK + private const string NamedSlotName = "Datadog_IISPreInitStart"; + private static bool _isIisPreStartInitComplete; + static CallTargetInvoker() { - // The first time the CallTargetInvoker is called - // we ensure that the non native parts of the initialization ran - // This is required for AOT scenarios where there is no clrprofiler - // to inject and run the loader. - Instrumentation.InitializeNoNativeParts(); + // Check if IIS automatic instrumentation has set the AppDomain property to indicate the PreStartInit state + // This is added to the startup hook by the CorProfiler in CorProfiler::AddIISPreStartInitFlags() + // which sets the value to `false` when the InvokePreStartInitMethods() method starts, and sets it to + // `true` after it's finished. Only once it returns to `true` can we start running CallTarget integrations. + var state = AppDomain.CurrentDomain.GetData(NamedSlotName); + if (state is bool boolState) + { + // we know we must be in IIS, so we need to check the app domain state + _isIisPreStartInitComplete = !boolState; + } + else + { + // We could _either_ not be in IIS, or we could be in an IIS app domain that hasn't set the property yet + // This can happen when we have multiple apps running in the same app domain. + try + { + // We need to check if we're running in IIS, so that we know whether to _expect_ + // the IIS PreStartInit AppDomain property to be set. Outside of IIS, it will never be set. + // We can't use ProcessHelpers here, because that could cause premature initialization of the + // tracer, which could cause recursion issues with IIS PreStartInit code execution + var processName = Process.GetCurrentProcess().ProcessName; + + if (processName.Equals("w3wp", StringComparison.OrdinalIgnoreCase) || + processName.Equals("iisexpress", StringComparison.OrdinalIgnoreCase)) + { + // We're in IIS, so we know we need to check the AppDomain property + // In previous workarounds for similar issues (e.g. https://github.com/DataDog/dd-trace-dotnet/pull/1157) + // we resorted to checking the callstack to see if it contained + // System.Web.Hosting.HostingEnvironment.Initialize(). That's generally unnecessary here, as + // if we know we're in IIS, we know we should be initialised _eventually_. Plus, checking the + // stack doesn't always work anyway, because if a threadpool thread is injected with a calltarget + // instrumentation, all we see is `Dispatch()`, which is not helpful. + _isIisPreStartInitComplete = false; + } + else + { + // If we're not in IIS, we don't need to run any pre-init checks, so mark as already complete + _isIisPreStartInitComplete = true; + } + } + catch (Exception) + { + // Error getting process name, have to assume we _aren't_ in IIS, + // and that we don't need to wait for the app domain data + _isIisPreStartInitComplete = true; + } + } } +#endif /// /// Begin Method Invoker @@ -37,7 +92,7 @@ static CallTargetInvoker() [MethodImpl(MethodImplOptions.AggressiveInlining)] public static CallTargetState BeginMethod(TTarget? instance) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance); @@ -58,7 +113,7 @@ public static CallTargetState BeginMethod(TTarget? instan [MethodImpl(MethodImplOptions.AggressiveInlining)] public static CallTargetState BeginMethod(TTarget? instance, TArg1? arg1) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1); @@ -81,7 +136,7 @@ public static CallTargetState BeginMethod(TTarget? [MethodImpl(MethodImplOptions.AggressiveInlining)] public static CallTargetState BeginMethod(TTarget? instance, TArg1? arg1, TArg2? arg2) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2); @@ -106,7 +161,7 @@ public static CallTargetState BeginMethod(T [MethodImpl(MethodImplOptions.AggressiveInlining)] public static CallTargetState BeginMethod(TTarget? instance, TArg1? arg1, TArg2? arg2, TArg3? arg3) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3); @@ -133,7 +188,7 @@ public static CallTargetState BeginMethod(TTarget? instance, TArg1? arg1, TArg2? arg2, TArg3? arg3, TArg4? arg4) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3, ref arg4); @@ -162,7 +217,7 @@ public static CallTargetState BeginMethod(TTarget? instance, TArg1? arg1, TArg2? arg2, TArg3? arg3, TArg4? arg4, TArg5? arg5) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3, ref arg4, ref arg5); @@ -193,7 +248,7 @@ public static CallTargetState BeginMethod(TTarget? instance, TArg1? arg1, TArg2? arg2, TArg3? arg3, TArg4? arg4, TArg5? arg5, TArg6? arg6) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3, ref arg4, ref arg5, ref arg6); @@ -226,7 +281,7 @@ public static CallTargetState BeginMethod(TTarget? instance, TArg1? arg1, TArg2? arg2, TArg3? arg3, TArg4? arg4, TArg5? arg5, TArg6? arg6, TArg7? arg7) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3, ref arg4, ref arg5, ref arg6, ref arg7); @@ -261,7 +316,7 @@ public static CallTargetState BeginMethod(TTarget? instance, TArg1? arg1, TArg2? arg2, TArg3? arg3, TArg4? arg4, TArg5? arg5, TArg6? arg6, TArg7? arg7, TArg8? arg8) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3, ref arg4, ref arg5, ref arg6, ref arg7, ref arg8); @@ -282,7 +337,7 @@ public static CallTargetState BeginMethod(TTarget? instance, ref TArg1? arg1) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1); @@ -305,7 +360,7 @@ public static CallTargetState BeginMethod(TTarget? [MethodImpl(MethodImplOptions.AggressiveInlining)] public static CallTargetState BeginMethod(TTarget? instance, ref TArg1? arg1, ref TArg2? arg2) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2); @@ -330,7 +385,7 @@ public static CallTargetState BeginMethod(T [MethodImpl(MethodImplOptions.AggressiveInlining)] public static CallTargetState BeginMethod(TTarget? instance, ref TArg1? arg1, ref TArg2? arg2, ref TArg3? arg3) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3); @@ -357,7 +412,7 @@ public static CallTargetState BeginMethod(TTarget? instance, ref TArg1? arg1, ref TArg2? arg2, ref TArg3? arg3, ref TArg4? arg4) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3, ref arg4); @@ -386,7 +441,7 @@ public static CallTargetState BeginMethod(TTarget? instance, ref TArg1? arg1, ref TArg2? arg2, ref TArg3? arg3, ref TArg4? arg4, ref TArg5? arg5) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3, ref arg4, ref arg5); @@ -417,7 +472,7 @@ public static CallTargetState BeginMethod(TTarget? instance, ref TArg1? arg1, ref TArg2? arg2, ref TArg3? arg3, ref TArg4? arg4, ref TArg5? arg5, ref TArg6? arg6) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3, ref arg4, ref arg5, ref arg6); @@ -450,7 +505,7 @@ public static CallTargetState BeginMethod(TTarget? instance, ref TArg1? arg1, ref TArg2? arg2, ref TArg3? arg3, ref TArg4? arg4, ref TArg5? arg5, ref TArg6? arg6, ref TArg7? arg7) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3, ref arg4, ref arg5, ref arg6, ref arg7); @@ -485,7 +540,7 @@ public static CallTargetState BeginMethod(TTarget? instance, ref TArg1? arg1, ref TArg2? arg2, ref TArg3? arg3, ref TArg4? arg4, ref TArg5? arg5, ref TArg6? arg6, ref TArg7? arg7, ref TArg8? arg8) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodHandler.Invoke(instance, ref arg1, ref arg2, ref arg3, ref arg4, ref arg5, ref arg6, ref arg7, ref arg8); @@ -505,7 +560,7 @@ public static CallTargetState BeginMethod(TTarget? instance, object[] arguments) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return BeginMethodSlowHandler.Invoke(instance, arguments); @@ -526,7 +581,7 @@ public static CallTargetState BeginMethod(TTarget? instan [MethodImpl(MethodImplOptions.AggressiveInlining)] public static CallTargetReturn EndMethod(TTarget? instance, Exception? exception, CallTargetState state) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); EndMethodHandler.Invoke(instance, exception, in state); @@ -549,7 +604,7 @@ public static CallTargetReturn EndMethod(TTarget? instanc [MethodImpl(MethodImplOptions.AggressiveInlining)] public static CallTargetReturn EndMethod(TTarget? instance, TReturn? returnValue, Exception? exception, CallTargetState state) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); var result = EndMethodHandler.Invoke(instance, returnValue, exception, in state); @@ -571,7 +626,7 @@ public static CallTargetReturn EndMethod(TTarget? instance, Exception? exception, in CallTargetState state) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return EndMethodHandler.Invoke(instance, exception, in state); @@ -594,7 +649,7 @@ public static CallTargetReturn EndMethod(TTarget? instanc [MethodImpl(MethodImplOptions.AggressiveInlining)] public static CallTargetReturn EndMethod(TTarget? instance, TReturn? returnValue, Exception? exception, in CallTargetState state) { - if (IntegrationOptions.IsIntegrationEnabled) + if (CanExecuteCallTargetIntegration() && IntegrationOptions.IsIntegrationEnabled) { IntegrationOptions.RecordTelemetry(); return EndMethodHandler.Invoke(instance, returnValue, exception, in state); @@ -612,7 +667,15 @@ public static CallTargetReturn EndMethod(Exception exception) { - IntegrationOptions.LogException(exception); + // Not calling CanExecuteCallTargetIntegration because that allows execution + // in some scenarios that we definitely _shouldn't_ be running here, so + // strictly checking _isIisPreStartInitComplete instead. +#if NETFRAMEWORK + if (_isIisPreStartInitComplete) +#endif + { + IntegrationOptions.LogException(exception); + } } /// @@ -634,4 +697,29 @@ public static unsafe CallTargetRefStruct CreateRefStruct(void* refStructPointer, { return CallTargetRefStruct.Create(refStructPointer, refStructTypeHandle); } + +#if NETFRAMEWORK + private static bool CanExecuteCallTargetIntegration([CallerMemberName] string callerName = null!) + { + if (_isIisPreStartInitComplete) + { + return true; + } + + var boolState = AppDomain.CurrentDomain.GetData(NamedSlotName); + _isIisPreStartInitComplete = boolState is false; + + // We _have_ to allow the HttpModule_Integration invocation through, even if we're in the Iis PreStart phase. + // That integration is specifically designed to run in this phase. We considered other options + // such as moving it to Instrumentation.Initialise, or rewriting directly with the profiling API + // but this was the simplest, easiest, and safest approach we could see generally. + var returnValue = _isIisPreStartInitComplete || typeof(TIntegration) == typeof(HttpModule_Integration); + return returnValue; + } + +#else + // Compiler should inline this out of the condition checks completely + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool CanExecuteCallTargetIntegration() => true; +#endif }