Skip to content

Commit

Permalink
fix(context): do not modify stack array directly when attach and detach
Browse files Browse the repository at this point in the history
  • Loading branch information
tientt-holistics committed Nov 14, 2024
1 parent 555b062 commit cd841ef
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 3 deletions.
7 changes: 4 additions & 3 deletions api/lib/opentelemetry/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand Down
55 changes: 55 additions & 0 deletions api/test/opentelemetry/context_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
t = Thread.new do
t2 = Thread.current
locals.each { |k, v| t2[k] = v }

Context.attach(ctx)
done_attach_new_context = true
end

while !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)
t = Thread.new do
t2 = Thread.current
locals.each { |k, v| t2[k] = v }

Context.clear
done_clear_context = true
end

while !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

0 comments on commit cd841ef

Please sign in to comment.