Skip to content

Commit

Permalink
fix threading issue in TracingHttpModule (#655)
Browse files Browse the repository at this point in the history
  • Loading branch information
lucaspimentel authored Feb 21, 2020
1 parent 12f2f6b commit b43d97a
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
7 changes: 4 additions & 3 deletions samples-aspnet/Samples.AspNetMvc5/Web.config
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,19 @@
<provider invariantName="System.Data.SqlClient" type="System.Data.Entity.SqlServer.SqlProviderServices, EntityFramework.SqlServer" />
</providers>
</entityFramework>

<system.codedom>
<compilers>
<compiler language="c#;cs;csharp" extension=".cs" type="Microsoft.CodeDom.Providers.DotNetCompilerPlatform.CSharpCodeProvider, Microsoft.CodeDom.Providers.DotNetCompilerPlatform, Version=2.0.1.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" warningLevel="4" compilerOptions="/langversion:6 /nowarn:1659;1699;1701" />
<compiler language="vb;vbs;visualbasic;vbscript" extension=".vb" type="Microsoft.CodeDom.Providers.DotNetCompilerPlatform.VBCodeProvider, Microsoft.CodeDom.Providers.DotNetCompilerPlatform, Version=2.0.1.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" warningLevel="4" compilerOptions="/langversion:14 /nowarn:41008 /define:_MYTYPE=\&quot;Web\&quot; /optionInfer+" />
</compilers>
</system.codedom>
<system.webServer>
<system.webServer>
<handlers>
<remove name="ExtensionlessUrlHandler-Integrated-4.0" />
<remove name="OPTIONSVerbHandler" />
<remove name="TRACEVerbHandler" />
<add name="ExtensionlessUrlHandler-Integrated-4.0" path="*." verb="*" type="System.Web.Handlers.TransferRequestHandler" preCondition="integratedMode,runtimeVersionv4.0" />
</handlers>
</system.webServer></configuration>
</system.webServer>
</configuration>
18 changes: 12 additions & 6 deletions src/Datadog.Trace.AspNet/TracingHttpModule.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System;
using System.Collections.Generic;
using System.Collections.Concurrent;
using System.Web;
using Datadog.Trace.ExtensionMethods;
using Datadog.Trace.Logging;
Expand All @@ -19,7 +19,9 @@ public class TracingHttpModule : IHttpModule

private static readonly Vendors.Serilog.ILogger Log = DatadogLogging.GetLogger(typeof(TracingHttpModule));

private static HashSet<HttpApplication> registeredEventHandlers = new HashSet<HttpApplication>();
// there is no ConcurrentHashSet, so use a ConcurrentDictionary
// where we only care about the key, not the value
private static ConcurrentDictionary<HttpApplication, byte> registeredEventHandlers = new ConcurrentDictionary<HttpApplication, byte>();

private readonly string _httpContextScopeKey;
private readonly string _requestOperationName;
Expand Down Expand Up @@ -56,7 +58,7 @@ public void Init(HttpApplication httpApplication)
// I've discovered that not letting each of these unique application objects be added, and thus
// the event handlers be registered within each HttpApplication object, leads to the runtime
// weirdness: at one point it crashed consistently for me, and later, I saw no spans at all.
if (registeredEventHandlers.Add(httpApplication))
if (registeredEventHandlers.TryAdd(httpApplication, 1))
{
_httpApplication = httpApplication;
httpApplication.BeginRequest += OnBeginRequest;
Expand All @@ -68,9 +70,13 @@ public void Init(HttpApplication httpApplication)
/// <inheritdoc />
public void Dispose()
{
// Remove the HttpApplication mapping so we don't keep the object alive
registeredEventHandlers.Remove(_httpApplication);
_httpApplication = null;
// defend against multiple calls to Dispose()
if (_httpApplication != null)
{
// Remove the HttpApplication mapping so we don't keep the object alive
registeredEventHandlers.TryRemove(_httpApplication, out var _);
_httpApplication = null;
}
}

private void OnBeginRequest(object sender, EventArgs eventArgs)
Expand Down

0 comments on commit b43d97a

Please sign in to comment.