From a96524beb7644569124cf72d52f76d5e57151f83 Mon Sep 17 00:00:00 2001
From: Stephen Toub <stoub@microsoft.com>
Date: Tue, 20 Aug 2019 15:21:27 -0400
Subject: [PATCH] Avoid race condition in DiagnosticCounter
 (dotnet/coreclr#26260)

* Avoid race condition in DiagnosticCounter

If while a DiagnosticCounter instance is being created the timer fires to process counters, the timer may end up evaluating a DiagnosticCounter whose constructor or derived types' constructor hasn't finished configuring the instance's state yet.

* Remove unnecessary #if


Commit migrated from https://github.com/dotnet/coreclr/commit/5ae4f6d650b001f61961316206b373c9c71e5fe5
---
 .../Diagnostics/Tracing/DiagnosticCounter.cs       | 22 +++++++++++++++++-----
 .../src/System/Diagnostics/Tracing/EventCounter.cs |  1 +
 .../Tracing/IncrementingEventCounter.cs            |  1 +
 .../Tracing/IncrementingPollingCounter.cs          |  1 +
 .../System/Diagnostics/Tracing/PollingCounter.cs   |  1 +
 5 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/DiagnosticCounter.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/DiagnosticCounter.cs
index a730392..c903686 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/DiagnosticCounter.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/DiagnosticCounter.cs
@@ -40,13 +40,25 @@ namespace System.Diagnostics.Tracing
                 throw new ArgumentNullException(nameof(EventSource));
             }
 
-            _group = CounterGroup.GetCounterGroup(eventSource);
-            _group.Add(this);
             Name = name;
-            DisplayUnits = string.Empty;
             EventSource = eventSource;
         }
 
+        /// <summary>Adds the counter to the set that the EventSource will report on.</summary>
+        /// <remarks>
+        /// Must only be invoked once, and only after the instance has been fully initialized.
+        /// This should be invoked by a derived type's ctor as the last thing it does.
+        /// </remarks>
+        private protected void Publish()
+        {
+            Debug.Assert(_group is null);
+            Debug.Assert(Name != null);
+            Debug.Assert(EventSource != null);
+
+            _group = CounterGroup.GetCounterGroup(EventSource);
+            _group.Add(this);
+        }
+
         /// <summary>
         /// Removes the counter from set that the EventSource will report on.  After being disposed, this
         /// counter will do nothing and its resource will be reclaimed if all references to it are removed.
@@ -58,7 +70,7 @@ namespace System.Diagnostics.Tracing
             if (_group != null)
             {
                 _group.Remove(this);
-                _group = null!; // TODO-NULLABLE: Avoid nulling out in Dispose
+                _group = null;
             }
         }
 
@@ -104,7 +116,7 @@ namespace System.Diagnostics.Tracing
 
         #region private implementation
 
-        private CounterGroup _group;
+        private CounterGroup? _group;
         private Dictionary<string, string?>? _metadata;
 
         internal abstract void WritePayload(float intervalSec, int pollingIntervalMillisec);
diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventCounter.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventCounter.cs
index 2cad56a..b4a15e5 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventCounter.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventCounter.cs
@@ -38,6 +38,7 @@ namespace System.Diagnostics.Tracing
             _max = double.NegativeInfinity;
 
             InitializeBuffer();
+            Publish();
         }
 
         /// <summary>
diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/IncrementingEventCounter.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/IncrementingEventCounter.cs
index ee54cfd..1fd66cd 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/IncrementingEventCounter.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/IncrementingEventCounter.cs
@@ -30,6 +30,7 @@ namespace System.Diagnostics.Tracing
         /// <param name="eventSource">The event source.</param>
         public IncrementingEventCounter(string name, EventSource eventSource) : base(name, eventSource)
         {
+            Publish();
         }
 
         /// <summary>
diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/IncrementingPollingCounter.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/IncrementingPollingCounter.cs
index 48eb0b0..6495180 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/IncrementingPollingCounter.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/IncrementingPollingCounter.cs
@@ -35,6 +35,7 @@ namespace System.Diagnostics.Tracing
                 throw new ArgumentNullException(nameof(totalValueProvider));
 
             _totalValueProvider = totalValueProvider;
+            Publish();
         }
 
         public override string ToString() => $"IncrementingPollingCounter '{Name}' Increment {_increment}";
diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/PollingCounter.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/PollingCounter.cs
index d578b56..29e11a5 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/PollingCounter.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/PollingCounter.cs
@@ -33,6 +33,7 @@ namespace System.Diagnostics.Tracing
                 throw new ArgumentNullException(nameof(metricProvider));
 
             _metricProvider = metricProvider;
+            Publish();
         }
 
         public override string ToString() => $"PollingCounter '{Name}' Count {1} Mean {_lastVal.ToString("n3")}";
-- 
2.7.4