Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(context): do not modify stack array directly when attach and detach #1760

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
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
Loading