Skip to content

Commit 0512a92

Browse files
dramos020carlossanlop
dramos020
and
carlossanlop
authored
[release/7.0] Starting metrics before runtime start leads to a crash (#81308)
* Fixes issue when monitoring a process launched via the same command line (#76965) * Initial fix for null reference exception * Took away lazy and put back private constructor * Added parent property and made handler thread safe * updated servicing version --------- Co-authored-by: Carlos Sanchez <[email protected]>
1 parent e9d6af0 commit 0512a92

File tree

2 files changed

+53
-36
lines changed

2 files changed

+53
-36
lines changed

‎src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
Commonly Used Types:
1313
System.Diagnostics.DiagnosticListener
1414
System.Diagnostics.DiagnosticSource</PackageDescription>
15-
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
16-
<ServicingVersion>1</ServicingVersion>
15+
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
16+
<ServicingVersion>2</ServicingVersion>
1717
</PropertyGroup>
1818

1919
<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->

‎src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs

Lines changed: 51 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Globalization;
88
using System.Runtime.Versioning;
99
using System.Text;
10+
using System.Threading;
1011

1112
namespace System.Diagnostics.Metrics
1213
{
@@ -60,13 +61,22 @@ public static class Keywords
6061
public const EventKeywords InstrumentPublishing = (EventKeywords)0x4;
6162
}
6263

63-
private CommandHandler _handler;
64+
private CommandHandler? _handler;
6465

65-
private MetricsEventSource()
66+
private CommandHandler Handler
6667
{
67-
_handler = new CommandHandler();
68+
get
69+
{
70+
if (_handler == null)
71+
{
72+
Interlocked.CompareExchange(ref _handler, new CommandHandler(this), null);
73+
}
74+
return _handler;
75+
}
6876
}
6977

78+
private MetricsEventSource() { }
79+
7080
/// <summary>
7181
/// Used to send ad-hoc diagnostics to humans.
7282
/// </summary>
@@ -189,7 +199,7 @@ protected override void OnEventCommand(EventCommandEventArgs command)
189199
{
190200
lock (this)
191201
{
192-
_handler.OnEventCommand(command);
202+
Handler.OnEventCommand(command);
193203
}
194204
}
195205

@@ -202,6 +212,13 @@ private sealed class CommandHandler
202212
private AggregationManager? _aggregationManager;
203213
private string _sessionId = "";
204214

215+
public CommandHandler(MetricsEventSource parent)
216+
{
217+
Parent = parent;
218+
}
219+
220+
public MetricsEventSource Parent { get; private set;}
221+
205222
public void OnEventCommand(EventCommandEventArgs command)
206223
{
207224
try
@@ -215,7 +232,7 @@ public void OnEventCommand(EventCommandEventArgs command)
215232
// This limitation shouldn't really matter because browser also doesn't support out-of-proc EventSource communication
216233
// which is the intended scenario for this EventSource. If it matters in the future AggregationManager can be
217234
// modified to have some other fallback path that works for browser.
218-
Log.Error("", "System.Diagnostics.Metrics EventSource not supported on browser");
235+
Parent.Error("", "System.Diagnostics.Metrics EventSource not supported on browser");
219236
return;
220237
}
221238
#endif
@@ -233,13 +250,13 @@ public void OnEventCommand(EventCommandEventArgs command)
233250
// one other listener is active. In the future we might be able to figure out how
234251
// to infer the changes from the info we do have or add a better API but for now
235252
// I am taking the simple route and not supporting it.
236-
Log.MultipleSessionsNotSupportedError(_sessionId);
253+
Parent.MultipleSessionsNotSupportedError(_sessionId);
237254
return;
238255
}
239256

240257
_aggregationManager.Dispose();
241258
_aggregationManager = null;
242-
Log.Message($"Previous session with id {_sessionId} is stopped");
259+
Parent.Message($"Previous session with id {_sessionId} is stopped");
243260
}
244261
_sessionId = "";
245262
}
@@ -249,12 +266,12 @@ public void OnEventCommand(EventCommandEventArgs command)
249266
if (command.Arguments!.TryGetValue("SessionId", out string? id))
250267
{
251268
_sessionId = id!;
252-
Log.Message($"SessionId argument received: {_sessionId}");
269+
Parent.Message($"SessionId argument received: {_sessionId}");
253270
}
254271
else
255272
{
256273
_sessionId = System.Guid.NewGuid().ToString();
257-
Log.Message($"New session started. SessionId auto-generated: {_sessionId}");
274+
Parent.Message($"New session started. SessionId auto-generated: {_sessionId}");
258275
}
259276

260277

@@ -263,55 +280,55 @@ public void OnEventCommand(EventCommandEventArgs command)
263280
double refreshIntervalSecs;
264281
if (command.Arguments!.TryGetValue("RefreshInterval", out string? refreshInterval))
265282
{
266-
Log.Message($"RefreshInterval argument received: {refreshInterval}");
283+
Parent.Message($"RefreshInterval argument received: {refreshInterval}");
267284
if (!double.TryParse(refreshInterval, out refreshIntervalSecs))
268285
{
269-
Log.Message($"Failed to parse RefreshInterval. Using default {defaultIntervalSecs}s.");
286+
Parent.Message($"Failed to parse RefreshInterval. Using default {defaultIntervalSecs}s.");
270287
refreshIntervalSecs = defaultIntervalSecs;
271288
}
272289
else if (refreshIntervalSecs < AggregationManager.MinCollectionTimeSecs)
273290
{
274-
Log.Message($"RefreshInterval too small. Using minimum interval {AggregationManager.MinCollectionTimeSecs} seconds.");
291+
Parent.Message($"RefreshInterval too small. Using minimum interval {AggregationManager.MinCollectionTimeSecs} seconds.");
275292
refreshIntervalSecs = AggregationManager.MinCollectionTimeSecs;
276293
}
277294
}
278295
else
279296
{
280-
Log.Message($"No RefreshInterval argument received. Using default {defaultIntervalSecs}s.");
297+
Parent.Message($"No RefreshInterval argument received. Using default {defaultIntervalSecs}s.");
281298
refreshIntervalSecs = defaultIntervalSecs;
282299
}
283300

284301
int defaultMaxTimeSeries = 1000;
285302
int maxTimeSeries;
286303
if (command.Arguments!.TryGetValue("MaxTimeSeries", out string? maxTimeSeriesString))
287304
{
288-
Log.Message($"MaxTimeSeries argument received: {maxTimeSeriesString}");
305+
Parent.Message($"MaxTimeSeries argument received: {maxTimeSeriesString}");
289306
if (!int.TryParse(maxTimeSeriesString, out maxTimeSeries))
290307
{
291-
Log.Message($"Failed to parse MaxTimeSeries. Using default {defaultMaxTimeSeries}");
308+
Parent.Message($"Failed to parse MaxTimeSeries. Using default {defaultMaxTimeSeries}");
292309
maxTimeSeries = defaultMaxTimeSeries;
293310
}
294311
}
295312
else
296313
{
297-
Log.Message($"No MaxTimeSeries argument received. Using default {defaultMaxTimeSeries}");
314+
Parent.Message($"No MaxTimeSeries argument received. Using default {defaultMaxTimeSeries}");
298315
maxTimeSeries = defaultMaxTimeSeries;
299316
}
300317

301318
int defaultMaxHistograms = 20;
302319
int maxHistograms;
303320
if (command.Arguments!.TryGetValue("MaxHistograms", out string? maxHistogramsString))
304321
{
305-
Log.Message($"MaxHistograms argument received: {maxHistogramsString}");
322+
Parent.Message($"MaxHistograms argument received: {maxHistogramsString}");
306323
if (!int.TryParse(maxHistogramsString, out maxHistograms))
307324
{
308-
Log.Message($"Failed to parse MaxHistograms. Using default {defaultMaxHistograms}");
325+
Parent.Message($"Failed to parse MaxHistograms. Using default {defaultMaxHistograms}");
309326
maxHistograms = defaultMaxHistograms;
310327
}
311328
}
312329
else
313330
{
314-
Log.Message($"No MaxHistogram argument received. Using default {defaultMaxHistograms}");
331+
Parent.Message($"No MaxHistogram argument received. Using default {defaultMaxHistograms}");
315332
maxHistograms = defaultMaxHistograms;
316333
}
317334

@@ -320,27 +337,27 @@ public void OnEventCommand(EventCommandEventArgs command)
320337
maxTimeSeries,
321338
maxHistograms,
322339
(i, s) => TransmitMetricValue(i, s, sessionId),
323-
(startIntervalTime, endIntervalTime) => Log.CollectionStart(sessionId, startIntervalTime, endIntervalTime),
324-
(startIntervalTime, endIntervalTime) => Log.CollectionStop(sessionId, startIntervalTime, endIntervalTime),
325-
i => Log.BeginInstrumentReporting(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
326-
i => Log.EndInstrumentReporting(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
327-
i => Log.InstrumentPublished(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
328-
() => Log.InitialInstrumentEnumerationComplete(sessionId),
329-
e => Log.Error(sessionId, e.ToString()),
330-
() => Log.TimeSeriesLimitReached(sessionId),
331-
() => Log.HistogramLimitReached(sessionId),
332-
e => Log.ObservableInstrumentCallbackError(sessionId, e.ToString()));
340+
(startIntervalTime, endIntervalTime) => Parent.CollectionStart(sessionId, startIntervalTime, endIntervalTime),
341+
(startIntervalTime, endIntervalTime) => Parent.CollectionStop(sessionId, startIntervalTime, endIntervalTime),
342+
i => Parent.BeginInstrumentReporting(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
343+
i => Parent.EndInstrumentReporting(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
344+
i => Parent.InstrumentPublished(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
345+
() => Parent.InitialInstrumentEnumerationComplete(sessionId),
346+
e => Parent.Error(sessionId, e.ToString()),
347+
() => Parent.TimeSeriesLimitReached(sessionId),
348+
() => Parent.HistogramLimitReached(sessionId),
349+
e => Parent.ObservableInstrumentCallbackError(sessionId, e.ToString()));
333350

334351
_aggregationManager.SetCollectionPeriod(TimeSpan.FromSeconds(refreshIntervalSecs));
335352

336353
if (command.Arguments!.TryGetValue("Metrics", out string? metricsSpecs))
337354
{
338-
Log.Message($"Metrics argument received: {metricsSpecs}");
355+
Parent.Message($"Metrics argument received: {metricsSpecs}");
339356
ParseSpecs(metricsSpecs);
340357
}
341358
else
342359
{
343-
Log.Message("No Metrics argument received");
360+
Parent.Message("No Metrics argument received");
344361
}
345362

346363
_aggregationManager.Start();
@@ -354,7 +371,7 @@ public void OnEventCommand(EventCommandEventArgs command)
354371

355372
private bool LogError(Exception e)
356373
{
357-
Log.Error(_sessionId, e.ToString());
374+
Parent.Error(_sessionId, e.ToString());
358375
// this code runs as an exception filter
359376
// returning false ensures the catch handler isn't run
360377
return false;
@@ -374,11 +391,11 @@ private void ParseSpecs(string? metricsSpecs)
374391
{
375392
if (!MetricSpec.TryParse(specString, out MetricSpec spec))
376393
{
377-
Log.Message($"Failed to parse metric spec: {specString}");
394+
Parent.Message($"Failed to parse metric spec: {specString}");
378395
}
379396
else
380397
{
381-
Log.Message($"Parsed metric: {spec}");
398+
Parent.Message($"Parsed metric: {spec}");
382399
if (spec.InstrumentName != null)
383400
{
384401
_aggregationManager!.Include(spec.MeterName, spec.InstrumentName);

0 commit comments

Comments
 (0)