From 4058a9143e5f45611fc4cc852b378f9f6fb6d34f Mon Sep 17 00:00:00 2001 From: TienTT Date: Thu, 14 Nov 2024 16:42:35 +0700 Subject: [PATCH] fix(context): do not modify stack array directly when attach and detach --- api/lib/opentelemetry/context.rb | 7 ++-- api/test/opentelemetry/context_test.rb | 55 ++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/api/lib/opentelemetry/context.rb b/api/lib/opentelemetry/context.rb index 05e96ed6b..f6f9d3d95 100644 --- a/api/lib/opentelemetry/context.rb +++ b/api/lib/opentelemetry/context.rb @@ -41,8 +41,9 @@ def current # @return [Object] A token to be used when detaching def attach(context) s = stack - s.push(context) - s.size + new_stack = s + [context] + Thread.current[STACK_KEY] = new_stack + new_stack.size end # Restores the previous Context associated with the current Fiber. @@ -57,7 +58,7 @@ def detach(token) calls_matched = (token == s.size) OpenTelemetry.handle_error(exception: DetachError.new('calls to detach should match corresponding calls to attach.')) unless calls_matched - s.pop + Thread.current[STACK_KEY] = s[...-1] || [] calls_matched end diff --git a/api/test/opentelemetry/context_test.rb b/api/test/opentelemetry/context_test.rb index eb91a907b..a85a85787 100644 --- a/api/test/opentelemetry/context_test.rb +++ b/api/test/opentelemetry/context_test.rb @@ -331,5 +331,60 @@ _(Context.current[bar_key]).must_be_nil end end + + describe 'when current thread local variable get copied to new thread' do + # NOTE: this is the similar behavior of ActionController::Live as identified in open-telemetry/opentelemetry-ruby-contrib#772 + it 'attach and detach in child thread will not affect parent thread' do + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + ctx = new_context + + t1 = Thread.current + locals = t1.keys.map { |key| [key, t1[key]] } + + done_attach_new_context = false + + token1 = Context.attach(ctx) + Thread.new do + t2 = Thread.current + locals.each { |k, v| t2[k] = v } + + Context.attach(ctx) + done_attach_new_context = true + end + + until done_attach_new_context; end + + Context.detach(token1) + + _(log_stream.string).wont_match(/OpenTelemetry error: calls to detach should match corresponding calls to attach/) + end + end + + it 'clear in child thread will not affect parent thread' do + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + ctx = new_context + + t1 = Thread.current + locals = t1.keys.map { |key| [key, t1[key]] } + + done_clear_context = false + + token1 = Context.attach(ctx) + Thread.new do + t2 = Thread.current + locals.each { |k, v| t2[k] = v } + + Context.clear + done_clear_context = true + end + + until done_clear_context; end + + Context.detach(token1) + + _(log_stream.string).wont_match(/OpenTelemetry error: calls to detach should match corresponding calls to attach/) + end + end + end end end