From 259e134092eae57cb2ccbea028adc8e8d0fc7a19 Mon Sep 17 00:00:00 2001
From: Ivo Anjo <ivo.anjo@datadoghq.com>
Date: Tue, 2 Jul 2024 09:41:10 +0100
Subject: [PATCH] Add clarification for expected behavior of yield

---
 lib/datadog/profiling/ext/dir_monkey_patches.rb | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lib/datadog/profiling/ext/dir_monkey_patches.rb b/lib/datadog/profiling/ext/dir_monkey_patches.rb
index 9af75c406c8..efc34194055 100644
--- a/lib/datadog/profiling/ext/dir_monkey_patches.rb
+++ b/lib/datadog/profiling/ext/dir_monkey_patches.rb
@@ -6,6 +6,7 @@ module Profiling
     module Ext
       # All Ruby versions as of this writing have bugs in the dir class implementation, causing issues such as
       # https://github.com/DataDog/dd-trace-rb/issues/3450 .
+      # See also https://bugs.ruby-lang.org/issues/20586 for more details.
       #
       # This monkey patch for the Ruby `Dir` class works around these bugs for affected Ruby versions by temporarily
       # blocking the profiler from interrupting system calls.
@@ -43,6 +44,12 @@ def children(*args, &block)
             Datadog::Profiling::Collectors::CpuAndWallTimeWorker._native_resume_signals
           end
 
+          # NOTE: When wrapping methods that yield, it's OK if the `yield` raises an exception while signals are
+          # enabled. This is because:
+          # * We can call `_native_resume_signals` many times in a row, both because it's idempotent, as well as it's
+          #   very low overhead (see benchmarks/profiler_hold_resume_interruptions.rb)
+          # * When an exception is being raised, the iteration will stop anyway, so there's no longer a concern of a
+          #   signal causing Ruby to return an incorrect value
           def each_child(*args, &block)
             if block
               begin
@@ -80,6 +87,7 @@ def entries(*args, &block)
             Datadog::Profiling::Collectors::CpuAndWallTimeWorker._native_resume_signals
           end
 
+          # See note on methods that yield above.
           def foreach(*args, &block)
             if block
               begin
@@ -103,6 +111,7 @@ def foreach(*args, &block)
             end
           end
 
+          # See note on methods that yield above.
           def glob(*args, &block)
             if block
               begin
@@ -153,6 +162,7 @@ def children(*args, **kwargs, &block)
             Datadog::Profiling::Collectors::CpuAndWallTimeWorker._native_resume_signals
           end
 
+          # See note on methods that yield above.
           def each_child(*args, **kwargs, &block)
             if block
               begin
@@ -190,6 +200,7 @@ def entries(*args, **kwargs, &block)
             Datadog::Profiling::Collectors::CpuAndWallTimeWorker._native_resume_signals
           end
 
+          # See note on methods that yield above.
           def foreach(*args, **kwargs, &block)
             if block
               begin
@@ -213,6 +224,7 @@ def foreach(*args, **kwargs, &block)
             end
           end
 
+          # See note on methods that yield above.
           def glob(*args, **kwargs, &block)
             if block
               begin
@@ -251,6 +263,7 @@ def home(*args, **kwargs, &block)
       if RUBY_VERSION.start_with?('2.')
         # Monkey patches for Dir (Ruby 2 version). See DirMonkeyPatches above for more details.
         module DirInstanceMonkeyPatches
+          # See note on methods that yield above.
           def each(*args, &block)
             if block
               begin
@@ -274,6 +287,7 @@ def each(*args, &block)
           end
 
           unless RUBY_VERSION.start_with?('2.5.') # This is Ruby 2.6+
+            # See note on methods that yield above.
             def each_child(*args, &block)
               if block
                 begin
@@ -322,6 +336,7 @@ def pos(*args, &block)
       else
         # Monkey patches for Dir (Ruby 3 version). See DirMonkeyPatches above for more details.
         module DirInstanceMonkeyPatches
+          # See note on methods that yield above.
           def each(*args, **kwargs, &block)
             if block
               begin
@@ -344,6 +359,7 @@ def each(*args, **kwargs, &block)
             end
           end
 
+          # See note on methods that yield above.
           def each_child(*args, **kwargs, &block)
             if block
               begin