fix: avoid race condition in singleton instance method

Author: zvkemp

instrumentation/base/lib/opentelemetry/instrumentation/base.rb

@@ -69,8 +69,9 @@ class << self
           integer: ->(v) { v.is_a?(Integer) },
           string: ->(v) { v.is_a?(String) }
         }.freeze
+        SINGLETON_MUTEX = Thread::Mutex.new
 
-        private_constant :NAME_REGEX, :VALIDATORS
+        private_constant :NAME_REGEX, :VALIDATORS, :SINGLETON_MUTEX
 
         private :new
 
@@ -163,8 +164,10 @@ def option(name, default:, validate:)
         end
 
         def instance
-          @instance ||= new(instrumentation_name, instrumentation_version, install_blk,
-                            present_blk, compatible_blk, options)
+          @instance || SINGLETON_MUTEX.synchronize do
+            @instance ||= new(instrumentation_name, instrumentation_version, install_blk,
+                              present_blk, compatible_blk, options)
+          end
         end
 
         private

instrumentation/base/test/instrumentation/base_test.rb

@@ -59,9 +59,33 @@ def initialize(*args)
   end
 
   describe '.instance' do
+    let(:instrumentation) do
+      Class.new(OpenTelemetry::Instrumentation::Base) do
+        instrumentation_name 'test_instrumentation'
+        instrumentation_version '0.1.1'
+
+        def initialize(*args)
+          # Simulate latency by hinting the VM should switch tasks
+          # (this can also be accomplished by something like `sleep(0.1)`).
+          # This replicates the worst-case scenario when using default assignment
+          # to obtain a singleton, i.e. that the scheduler switches threads between
+          # the nil check and object initialization.
+          Thread.pass
+          super
+        end
+      end
+    end
+
     it 'returns an instance' do
       _(instrumentation.instance).must_be_instance_of(instrumentation)
     end
+
+    it 'returns the same singleton instance to every thread' do
+      object_ids = Array.new(2).map { Thread.new { instrumentation.instance } }
+                        .map { |thr| thr.join.value }
+
+      _(object_ids.uniq.count).must_equal(1)
+    end
   end
 
   describe '.option' do