fix: avoid race condition in singleton instance method

Author: zvkemp

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

@@ -60,6 +60,8 @@ module Instrumentation
     # '::' replaced by underscores, OPENTELEMETRY shortened to OTEL_{LANG}, and '_ENABLED' appended.
     # For example: OTEL_RUBY_INSTRUMENTATION_SINATRA_ENABLED = false.
     class Base
+      @singleton_mutex = Thread::Mutex.new
+
       class << self
         NAME_REGEX = /^(?:(?<namespace>[a-zA-Z0-9_:]+):{2})?(?<classname>[a-zA-Z0-9_]+)$/
         VALIDATORS = {
@@ -75,6 +77,7 @@ class << self
         private :new
 
         def inherited(subclass) # rubocop:disable Lint/MissingSuper
+          subclass.instance_exec { @singleton_mutex = Thread::Mutex.new }
           OpenTelemetry::Instrumentation.registry.register(subclass)
         end
 
@@ -163,8 +166,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,53 @@ 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
+
+    let(:other_instrumentation) do
+      aux_instrumentation = instrumentation
+      Class.new(OpenTelemetry::Instrumentation::Base) do
+        instrumentation_name 'test_instrumentation'
+        instrumentation_version '0.1.1'
+
+        define_method(:aux_instrumentation) { aux_instrumentation }
+
+        def initialize(*)
+          aux_instrumentation.instance
+
+          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
+
+    it 'can refer to other instances in initialize without deadlocking' do
+      other_instrumentation.instance
+    end
   end
 
   describe '.option' do