Skip to content

Commit

Permalink
Fix profiler not restarting after Ruby forks
Browse files Browse the repository at this point in the history
We already were testing if the worker thread was alive in native code
(which is why a different instance of the profiler could already be
started), but did not properly allow the same `CpuAndWallTimeWorker` to
be restarted.
  • Loading branch information
ivoanjo committed Nov 9, 2022
1 parent 13f7c9b commit b4b0807
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def initialize(

def start
@start_stop_mutex.synchronize do
return if @worker_thread
return if @worker_thread && @worker_thread.alive?

Datadog.logger.debug { "Starting thread for: #{self}" }
@worker_thread = Thread.new do
Expand Down
27 changes: 27 additions & 0 deletions spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,33 @@
current_thread_gc_samples.inject(0) { |sum, sample| sum + sample.fetch(:values).fetch(:'cpu-samples') }
).to be >= invoke_gc_times
end

context 'when the background thread dies without cleaning up (after Ruby forks)' do
it 'allows the CpuAndWallTimeWorker to be restarted' do
start

expect_in_fork do
cpu_and_wall_time_worker.start
wait_until_running
end
end

it 'allows a different instance of the CpuAndWallTimeWorker to be started' do
start

expect_in_fork do
another_instance = described_class.new(
recorder: Datadog::Profiling::StackRecorder.new,
max_frames: 400,
tracer: nil,
gc_profiling_enabled: gc_profiling_enabled,
)
another_instance.start

try_wait_until(backoff: 0.01) { described_class::Testing._native_is_running?(another_instance) }
end
end
end
end

describe 'Ractor safety' do
Expand Down

0 comments on commit b4b0807

Please sign in to comment.