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

Mark ddtrace threads as fork-safe #3279

Merged
merged 3 commits into from
Nov 27, 2023
Merged
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
4 changes: 4 additions & 0 deletions lib/datadog/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,10 @@ def logger_without_components
def handle_interrupt_shutdown!
logger = Datadog.logger
shutdown_thread = Thread.new { shutdown! }
unless Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.3')
shutdown_thread.name = Datadog::Core::Configuration.name
end

print_message_treshold_seconds = 0.2

slow_shutdown = shutdown_thread.join(print_message_treshold_seconds).nil?
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/core/remote/worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def start

thread = Thread.new { poll(@interval) }
thread.name = self.class.name unless Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.3')
thread.thread_variable_set(:fork_safe, true)
@thr = thread

@started = true
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/core/workers/async.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ def start_worker
# rubocop:enable Lint/RescueException
end
@worker.name = self.class.name unless Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.3')
@worker.thread_variable_set(:fork_safe, true)

nil
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def start(on_failure_proc: nil)
end
end
@worker_thread.name = self.class.name # Repeated from above to make sure thread gets named asap
@worker_thread.thread_variable_set(:fork_safe, true)
end

true
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/profiling/collectors/idle_sampling_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def start
end
end
@worker_thread.name = self.class.name # Repeated from above to make sure thread gets named asap
@worker_thread.thread_variable_set(:fork_safe, true)
end

true
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/tracing/workers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def start
Datadog.logger.debug { "Starting thread for: #{self}" }
@worker = Thread.new { perform }
@worker.name = self.class.name unless Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.3')
@worker.thread_variable_set(:fork_safe, true)

nil
end
Expand Down
8 changes: 7 additions & 1 deletion spec/datadog/core/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,13 @@
describe '#handle_interrupt_shutdown!' do
subject(:handle_interrupt_shutdown!) { test_class.send(:handle_interrupt_shutdown!) }

let(:fake_thread) { instance_double(Thread, 'fake thread') }
let(:fake_thread) do
instance_double(Thread, 'fake thread').tap do |it|
if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.3')
expect(it).to(receive(:name=).with('Datadog::Core::Configuration'))
end
end
end

it 'calls #shutdown! in a background thread' do
allow(fake_thread).to receive(:join).and_return(fake_thread)
Expand Down
7 changes: 7 additions & 0 deletions spec/datadog/core/remote/worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@
expect(Thread.list.map(&:name)).to include(described_class.to_s)
end
end

# See https://github.com/puma/puma/blob/32e011ab9e029c757823efb068358ed255fb7ef4/lib/puma/cluster.rb#L353-L359
it 'marks the worker thread as fork-safe (to avoid fork-safety warnings in webservers)' do
worker.start

expect(worker.instance_variable_get(:@thr).thread_variable_get(:fork_safe)).to be true
end
end

describe '#stop' do
Expand Down
9 changes: 8 additions & 1 deletion spec/datadog/core/workers/async_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@
end
end

describe 'thread naming' do
describe 'thread naming and fork-safety marker' do
after { worker.terminate }

context 'on Ruby < 2.3' do
Expand Down Expand Up @@ -488,6 +488,13 @@
expect(worker.send(:worker).name).to eq worker_class.to_s
end
end

# See https://github.com/puma/puma/blob/32e011ab9e029c757823efb068358ed255fb7ef4/lib/puma/cluster.rb#L353-L359
it 'marks the worker thread as fork-safe (to avoid fork-safety warnings in webservers)' do
worker.perform

expect(worker.send(:worker).thread_variable_get(:fork_safe)).to be true
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@
expect(Thread.list.map(&:name)).to include(described_class.name)
end

# See https://github.com/puma/puma/blob/32e011ab9e029c757823efb068358ed255fb7ef4/lib/puma/cluster.rb#L353-L359
it 'marks the new thread as fork-safe' do
start

expect(cpu_and_wall_time_worker.instance_variable_get(:@worker_thread).thread_variable_get(:fork_safe)).to be true
end

it 'does not create a second thread if start is called again' do
start

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@
start
end

# See https://github.com/puma/puma/blob/32e011ab9e029c757823efb068358ed255fb7ef4/lib/puma/cluster.rb#L353-L359
it 'marks the new thread as fork-safe' do
start

expect(idle_sampling_helper.instance_variable_get(:@worker_thread).thread_variable_get(:fork_safe)).to be true
end

it 'does not create a second thread if start is called again' do
start

Expand Down
9 changes: 8 additions & 1 deletion spec/datadog/tracing/workers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
end
end

describe 'thread naming' do
describe 'thread naming and fork-safety marker' do
context 'on Ruby < 2.3' do
before do
skip 'Only applies to old Rubies' if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.3')
Expand All @@ -65,6 +65,13 @@
expect(worker.instance_variable_get(:@worker).name).to eq described_class.name
end
end

# See https://github.com/puma/puma/blob/32e011ab9e029c757823efb068358ed255fb7ef4/lib/puma/cluster.rb#L353-L359
it 'marks the worker thread as fork-safe (to avoid fork-safety warnings in webservers)' do
worker.start

expect(worker.instance_variable_get(:@worker).thread_variable_get(:fork_safe)).to be true
end
end

describe '#start' do
Expand Down
Loading