Skip to content

Commit 70af77a

Browse files
committed
Dispose statsd client in the background
The statsd client does sync-over-async in the flush and dispose paths, which can lead to deadlocks and thread exhaustion. To work around that, we push the dispose to happen on a thread-pool thread instead, in the background
1 parent 201abfb commit 70af77a

File tree

3 files changed

+109
-86
lines changed

3 files changed

+109
-86
lines changed

tracer/src/Datadog.Trace/DogStatsd/StatsdManager.cs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
using System;
99
using System.Threading;
10+
using System.Threading.Tasks;
1011
using Datadog.Trace.Configuration;
1112
using Datadog.Trace.Logging;
1213
using Datadog.Trace.Vendors.StatsdClient;
@@ -24,15 +25,15 @@ internal sealed class StatsdManager : IStatsdManager
2425
private readonly IDisposable _settingSubscription;
2526
private int _isRequiredMask;
2627
private StatsdClientHolder? _current;
27-
private Func<IDogStatsd> _factory;
28+
private Func<StatsdClientHolder> _factory;
2829

2930
public StatsdManager(TracerSettings tracerSettings)
3031
: this(tracerSettings, CreateClient)
3132
{
3233
}
3334

3435
// Internal for testing
35-
internal StatsdManager(TracerSettings tracerSettings, Func<MutableSettings, ExporterSettings, IDogStatsd> statsdFactory)
36+
internal StatsdManager(TracerSettings tracerSettings, Func<MutableSettings, ExporterSettings, StatsdClientHolder> statsdFactory)
3637
{
3738
// The initial factory, assuming there's no updates
3839
_factory = () => statsdFactory(
@@ -172,8 +173,8 @@ internal static bool HasImpactingChanges(TracerSettings.SettingsManager.SettingC
172173
return hasChanges;
173174
}
174175

175-
private static IDogStatsd CreateClient(MutableSettings settings, ExporterSettings exporter)
176-
=> StatsdFactory.CreateDogStatsdClient(settings, exporter, includeDefaultTags: true);
176+
private static StatsdClientHolder CreateClient(MutableSettings settings, ExporterSettings exporter)
177+
=> new(StatsdFactory.CreateDogStatsdClient(settings, exporter, includeDefaultTags: true));
177178

178179
private void EnsureClient(bool ensureCreated, bool forceRecreate)
179180
{
@@ -189,9 +190,7 @@ private void EnsureClient(bool ensureCreated, bool forceRecreate)
189190
return;
190191
}
191192

192-
_current = ensureCreated
193-
? new StatsdClientHolder(_factory())
194-
: null;
193+
_current = ensureCreated ? _factory() : null;
195194
}
196195

197196
previous?.MarkClosing(); // will dispose when last lease releases
@@ -221,6 +220,9 @@ internal sealed class StatsdClientHolder(IDogStatsd client)
221220

222221
public IDogStatsd Client { get; } = client;
223222

223+
// Internal for testing
224+
public bool IsDisposed => Volatile.Read(ref _disposed) == 1;
225+
224226
public bool TryRetain()
225227
{
226228
while (true)
@@ -292,12 +294,18 @@ private void Dispose()
292294
if (Interlocked.Exchange(ref _disposed, 1) == 0)
293295
{
294296
Log.Debug("Disposing DogStatsdService");
295-
if (Client is DogStatsdService dogStatsd)
297+
298+
// We push this all to a background thread to avoid the disposes running in-line
299+
// the DogStatsdService does sync-over-async, and this can cause thread exhaustion
300+
_ = Task.Run(() =>
296301
{
297-
dogStatsd.Flush();
298-
}
302+
if (Client is DogStatsdService dogStatsd)
303+
{
304+
dogStatsd.Flush();
305+
}
299306

300-
Client.Dispose();
307+
Client.Dispose();
308+
});
301309
}
302310
}
303311
}

0 commit comments

Comments
 (0)