Improve thread-safety in GlobalTracer.IsRegistered (#119)

opentracing · Mar 28, 2019 · adcbfbf · adcbfbf
1 parent c534179
commit adcbfbf
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 11 deletions.
diff --git a/src/OpenTracing/Util/GlobalTracer.cs b/src/OpenTracing/Util/GlobalTracer.cs
@@ -65,15 +65,14 @@ public static bool IsRegistered()
         }
 
         /// <summary>
-        /// Register a <see cref="ITracer"/> to back the behaviour of the global tracer (<see cref="Instance"/>).
-        /// <para/>
-        /// Registration is a one-time operation, attempting to call it more often will result in a runtime exception.
-        /// <para/>
-        /// Every application intending to use the global tracer is responsible for registering it once
-        /// during its initialization.
+        /// Attempts to register a <see cref="ITracer"/> to back the behaviour of the global tracer (<see cref="Instance"/>).
+        /// <para>
+        /// Registration is a one-time operation, subsequent attempts do not change the registered global tracer.
+        /// </para>
         /// </summary>
-        /// <param name="tracer">Tracer to use as global tracer.</param>
-        public static void Register(ITracer tracer)
+        /// <param name="tracer">The tracer to use as the global tracer.</param>
+        /// <returns>True if the tracer was successfully registered, otherwise false.</returns>
+        public static bool RegisterIfAbsent(ITracer tracer)
         {
             if (tracer == null)
                 throw new ArgumentNullException(nameof(tracer), "Cannot register GlobalTracer <null>.");
@@ -83,17 +82,42 @@ public static void Register(ITracer tracer)
 
             lock (s_lock)
             {
+                // if re-registering the same tracer, just return true
                 if (tracer == s_instance._tracer)
                 {
                     s_isRegistered = true;
-                    return;
+                    return true;
                 }
 
-                if (IsRegistered())
-                    throw new InvalidOperationException("There is already a current global Tracer registered.");
+                // if a tracer is already registered, return false
+                if (s_isRegistered)
+                {
+                    return false;
+                }
 
                 s_instance._tracer = tracer;
                 s_isRegistered = true;
+
+                return true;
+            }
+        }
+
+        /// <summary>
+        /// Register a <see cref="ITracer"/> to back the behaviour of the global tracer (<see cref="Instance"/>).
+        /// <para/>
+        /// Registration is a one-time operation, attempting to call it more often will result in a runtime exception.
+        /// <para/>
+        /// Every application intending to use the global tracer is responsible for registering it once
+        /// during its initialization.
+        /// </summary>
+        /// <param name="tracer">Tracer to use as global tracer.</param>
+        public static void Register(ITracer tracer)
+        {
+            if (!RegisterIfAbsent(tracer) &&
+                tracer != s_instance._tracer &&
+                !(tracer is GlobalTracer))
+            {
+                throw new InvalidOperationException("There is already a current global Tracer registered.");
             }
         }
 

diff --git a/test/OpenTracing.Tests/Util/GlobalTracerTests.cs b/test/OpenTracing.Tests/Util/GlobalTracerTests.cs
@@ -126,5 +126,19 @@ public void IsRegistered_returns_true_after_registration()
 
             Assert.True(GlobalTracer.IsRegistered());
         }
+
+        [Fact]
+        public void RegisterIfAbsent_called_multiple_times_does_not_throw_exception()
+        {
+            Assert.False(GlobalTracer.IsRegistered());
+
+            for (var i = 0; i < 10; i++)
+            {
+                var tracer = Substitute.For<ITracer>();
+                GlobalTracer.RegisterIfAbsent(tracer);                
+
+                Assert.True(GlobalTracer.IsRegistered());
+            }
+        }
     }
 }