Skip to content

Commit

Permalink
Merge pull request #1537 from DataDog/ivoanjo/fix-name-collision-with…
Browse files Browse the repository at this point in the history
…-ruby-3_1

[PROF-3461] Rename Thread#native_thread_id to #pthread_thread_id to avoid conflict with Ruby 3.1
  • Loading branch information
ivoanjo authored Jun 3, 2021
2 parents 6f005d8 + 0cc71ac commit a801f69
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 18 deletions.
2 changes: 1 addition & 1 deletion lib/ddtrace/profiling/collectors/stack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def collect_thread_event(thread, wall_time_interval_ns)
# Convert backtrace locations into structs
locations = convert_backtrace_locations(locations)

thread_id = thread.respond_to?(:native_thread_id) ? thread.native_thread_id : thread.object_id
thread_id = thread.respond_to?(:pthread_thread_id) ? thread.pthread_thread_id : thread.object_id
trace_id, span_id = get_trace_identifiers(thread)
cpu_time = get_cpu_time_interval!(thread)

Expand Down
16 changes: 8 additions & 8 deletions lib/ddtrace/profiling/ext/cthread.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ module NativePthread
attach_function :pthread_getcpuclockid, [:ulong, CClockId], :int

# NOTE: Only returns thread ID for thread that evaluates this call.
# a.k.a. evaluating `get_native_thread_id(thread_a)` from within
# a.k.a. evaluating `get_pthread_thread_id(thread_a)` from within
# `thread_b` will return `thread_b`'s thread ID, not `thread_a`'s.
def self.get_native_thread_id(thread)
def self.get_pthread_thread_id(thread)
return unless ::Thread.current == thread

pthread_self
Expand Down Expand Up @@ -65,16 +65,16 @@ def self.prepended(base)
# it looks like there's some lazily-created structure that is missing and did not get created).
if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.3') &&
Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.7')
attr_reader :native_thread_id
attr_reader :pthread_thread_id
else
def native_thread_id
defined?(@native_thread_id) && @native_thread_id
def pthread_thread_id
defined?(@pthread_thread_id) && @pthread_thread_id
end
end

def initialize(*args)
@pid = ::Process.pid
@native_thread_id = nil
@pthread_thread_id = nil
@clock_id = nil

# Wrap the work block with our own
Expand Down Expand Up @@ -120,8 +120,8 @@ def update_native_ids
return unless ::Thread.current == self

@pid = ::Process.pid
@native_thread_id = NativePthread.get_native_thread_id(self)
@clock_id = NativePthread.get_clock_id(self, @native_thread_id)
@pthread_thread_id = NativePthread.get_pthread_thread_id(self)
@clock_id = NativePthread.get_clock_id(self, @pthread_thread_id)
end
end

Expand Down
18 changes: 9 additions & 9 deletions spec/ddtrace/profiling/ext/cthread_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def on_main_thread

context 'with a started thread' do
before do
# There is a brief period where a thread has been started but the native_thread_id and clock_id have not
# There is a brief period where a thread has been started but the pthread_thread_id and clock_id have not
# yet been set. This try_wait_until is here to ensure that we wait for them to be set before running any
# expectations, as otherwise this would generate test suite flakiness.
# The easiest way to simulate this is to add a `sleep(1)` to the start of `#update_native_ids`,
Expand All @@ -104,7 +104,7 @@ def on_main_thread
describe '::new' do
it 'has native thread IDs available' do
is_expected.to have_attributes(
native_thread_id: kind_of(Integer),
pthread_thread_id: kind_of(Integer),
cpu_time: kind_of(Float)
)
expect(thread.send(:clock_id)).to be_kind_of(Integer)
Expand All @@ -124,21 +124,21 @@ def on_main_thread
end
end

describe '#native_thread_id' do
subject(:native_thread_id) { thread.native_thread_id }
describe '#pthread_thread_id' do
subject(:pthread_thread_id) { thread.pthread_thread_id }

it { is_expected.to be_a_kind_of(Integer) }

context 'main thread' do
context 'when forked' do
it 'returns a new native thread ID' do
# Get main thread native ID
original_native_thread_id = thread.native_thread_id
original_pthread_thread_id = thread.pthread_thread_id

expect_in_fork do
# Expect main thread native ID to not change
expect(thread.native_thread_id).to be_a_kind_of(Integer)
expect(thread.native_thread_id).to eq(original_native_thread_id)
expect(thread.pthread_thread_id).to be_a_kind_of(Integer)
expect(thread.pthread_thread_id).to eq(original_pthread_thread_id)
end
end
end
Expand Down Expand Up @@ -293,10 +293,10 @@ def on_main_thread
end
end

describe '#native_thread_id' do
describe '#pthread_thread_id' do
it 'can be read without crashing the Ruby VM' do
with_profiling_extensions_in_fork do
expect(process_waiter_thread.native_thread_id).to be nil
expect(process_waiter_thread.pthread_thread_id).to be nil
end
end
end
Expand Down

0 comments on commit a801f69

Please sign in to comment.