From 3b9b3b8cff13bb3d7cf27f7d698f8a88809a61af Mon Sep 17 00:00:00 2001 From: st0012 Date: Wed, 6 Apr 2022 21:49:57 +0100 Subject: [PATCH] CheckBreakpoint should only stop at the first condition fulfillment line When a condition is fulfilled (e.g. a >= 1), it usually remains true for at least a while. However, under the current implementation, it'll stops at every single line after the condition becomes true, which is annoying. So this PR introduces an extra state to make sure the BP doesn't stop repeatedly after the condition is fulfilled. --- lib/debug/breakpoint.rb | 23 +++++++++++++++++++++-- lib/debug/thread_client.rb | 10 ++++++++++ test/debug/break_test.rb | 27 +++++++++++++++++++++------ 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/lib/debug/breakpoint.rb b/lib/debug/breakpoint.rb index 17d47b03a..8cb99f424 100644 --- a/lib/debug/breakpoint.rb +++ b/lib/debug/breakpoint.rb @@ -326,8 +326,13 @@ def setup next if ThreadClient.current.management? next if skip_path?(tp.path) - if safe_eval tp.binding, @cond - suspend + if safe_eval(tp.binding, @cond) + unless fulfilled_on_current_thread? + set_fulfilled_on_current_thread + suspend + end + else + set_unfulfilled_on_current_thread end } end @@ -337,6 +342,20 @@ def to_s s += super s end + + private + + def fulfilled_on_current_thread? + ThreadClient.current.check_bp_fulfilled?(self) + end + + def set_fulfilled_on_current_thread + ThreadClient.current.set_check_bp_state(self, true) + end + + def set_unfulfilled_on_current_thread + ThreadClient.current.set_check_bp_state(self, false) + end end class WatchIVarBreakpoint < Breakpoint diff --git a/lib/debug/thread_client.rb b/lib/debug/thread_client.rb index 98e38b19b..2d58f197f 100644 --- a/lib/debug/thread_client.rb +++ b/lib/debug/thread_client.rb @@ -95,6 +95,8 @@ def initialize id, q_evt, q_cmd, thr = Thread.current @obj_map = {} # { object_id => obj } for CDP @recorder = nil @mode = :waiting + # every thread should maintain its own CheckBreakpoint fulfillment state + @check_bp_fulfillment_map = {} # { check_bp => boolean } set_mode :running thr.instance_variable_set(:@__thread_client_id, id) @@ -337,6 +339,14 @@ def step_tp iter, events = [:line, :b_return, :return] end end + def set_check_bp_state(bp, state) + @check_bp_fulfillment_map[bp] = state + end + + def check_bp_fulfilled?(bp) + @check_bp_fulfillment_map[bp] + end + ## cmd helpers if TracePoint.respond_to? :allow_reentry diff --git a/test/debug/break_test.rb b/test/debug/break_test.rb index 059d7a0e9..872a1b794 100644 --- a/test/debug/break_test.rb +++ b/test/debug/break_test.rb @@ -645,8 +645,11 @@ def program 2| a += 1 3| a += 2 4| a += 3 - 5| - 6| binding.b + 5| a = 0 + 6| a = 1 + 7| a = 2 + 8| a = 3 + 9| binding.b RUBY end @@ -671,6 +674,22 @@ def test_conditional_breakpoint_shows_error end end + def test_conditional_breakpoint_ignores_unchanged_true_condition + debug_code(program) do + type 'break if: a > 0' + type 'c' + assert_line_num(2) # stopped by line 1 + # the state remains fulfilled so the next 2 lines don't trigger stoppage + type 'c' + # line 5 changes the state. so the next true condition will stop the program again + assert_line_num(7) # stopped by line 6 + type 'c' + # the state remains fulfilled again, so the line 7 doesn't trigger stoppage + assert_line_num(9) + type 'c' + end + end + class PathOptionTest < TestCase def extra_file <<~RUBY @@ -712,10 +731,6 @@ def test_the_path_option_supersede_skip_path_config assert_line_num(3) assert_line_text(/201/) type 'c' - # this stop is because check bp stops at every line after the condition is fulfilled, which is another issue - # see https://github.com/ruby/debug/issues/406 for more about the issue - assert_line_num(4) - type 'c' end end