From 0cc71ac80ced42d80a05324f7a1ea69451e4bb65 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Wed, 2 Jun 2021 09:49:24 +0100 Subject: [PATCH] Rename Thread#native_thread_id to #pthread_thread_id to avoid conflict with Ruby 3.1 Ruby 3.1 has introduced their own `Thread#native_thread_id` (see ) which differs from the `pthread_self()` we're using here. This means that our `CThread` monkey patch would conflict with the upstream behavior due to a name collision. To avoid this issue, I just did a global find-and-replace on the codebase to rename all usages of `#native_thread_id` with `#pthread_thread_id`. I have some ideas on how to get rid of the need to monkey patch `Thread` in the future, so hopefully at some point we'll just be able to delete these monkey patches. --- lib/ddtrace/profiling/collectors/stack.rb | 2 +- lib/ddtrace/profiling/ext/cthread.rb | 16 ++++++++-------- spec/ddtrace/profiling/ext/cthread_spec.rb | 18 +++++++++--------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/ddtrace/profiling/collectors/stack.rb b/lib/ddtrace/profiling/collectors/stack.rb index 9afcf590033..fc2b71b21f2 100644 --- a/lib/ddtrace/profiling/collectors/stack.rb +++ b/lib/ddtrace/profiling/collectors/stack.rb @@ -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) diff --git a/lib/ddtrace/profiling/ext/cthread.rb b/lib/ddtrace/profiling/ext/cthread.rb index 64e3a30f4ab..355f83a5697 100644 --- a/lib/ddtrace/profiling/ext/cthread.rb +++ b/lib/ddtrace/profiling/ext/cthread.rb @@ -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 @@ -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 @@ -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 diff --git a/spec/ddtrace/profiling/ext/cthread_spec.rb b/spec/ddtrace/profiling/ext/cthread_spec.rb index 9c900d1e011..13fbf88a541 100644 --- a/spec/ddtrace/profiling/ext/cthread_spec.rb +++ b/spec/ddtrace/profiling/ext/cthread_spec.rb @@ -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`, @@ -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) @@ -124,8 +124,8 @@ 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) } @@ -133,12 +133,12 @@ def on_main_thread 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 @@ -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