diff --git a/lib/debug/breakpoint.rb b/lib/debug/breakpoint.rb index 78c4d2174..1de5ccc0e 100644 --- a/lib/debug/breakpoint.rb +++ b/lib/debug/breakpoint.rb @@ -6,7 +6,7 @@ module DEBUGGER__ class Breakpoint include SkipPathHelper - attr_reader :key, :skip_src + attr_reader :key, :skip_src, :cond def initialize cond, command, path, do_enable: true @deleted = false @@ -19,6 +19,16 @@ def initialize cond, command, path, do_enable: true enable if do_enable end + # Returns a serializable hash for cross-process breakpoint sync, + # or nil if this breakpoint type is not syncable. + def to_sync_data + nil + end + + def syncable? + false + end + def safe_eval b, expr b.eval(expr) rescue Exception => e @@ -221,6 +231,16 @@ def activate_exact iseq, events, line end end + def to_sync_data + { 'type' => 'line', 'path' => @path, 'line' => @line, + 'cond' => @cond, 'oneshot' => @oneshot, + 'hook_call' => @hook_call, 'command' => @command } + end + + def syncable? + true + end + def duplicable? @oneshot end @@ -302,6 +322,15 @@ def path_is? path class CatchBreakpoint < Breakpoint attr_reader :last_exc + def to_sync_data + { 'type' => 'catch', 'pat' => @pat, 'cond' => @cond, + 'command' => @command, 'path' => @path } + end + + def syncable? + true + end + def initialize pat, cond: nil, command: nil, path: nil @pat = pat.freeze @key = [:catch, @pat].freeze @@ -427,6 +456,16 @@ def to_s class MethodBreakpoint < Breakpoint attr_reader :sig_method_name, :method, :klass + def to_sync_data + { 'type' => 'method', 'klass' => @sig_klass_name, + 'op' => @sig_op, 'method' => @sig_method_name, + 'cond' => @cond, 'command' => @command } + end + + def syncable? + true + end + def initialize b, klass_name, op, method_name, cond: nil, command: nil, path: nil @sig_klass_name = klass_name @sig_op = op diff --git a/lib/debug/server.rb b/lib/debug/server.rb index 0a3c7d266..5cafa9ef1 100644 --- a/lib/debug/server.rb +++ b/lib/debug/server.rb @@ -186,6 +186,9 @@ def process line = @session.process_group.sync do unless IO.select([@sock], nil, nil, 0) DEBUGGER__.debug{ "UI_Server can not read" } + # Wait briefly for the consuming process to publish breakpoint changes + sleep 0.05 + @session.bp_sync_check break :can_not_read end @sock.gets&.chomp.tap{|line| diff --git a/lib/debug/server_dap.rb b/lib/debug/server_dap.rb index 907db2d1f..7894b4356 100644 --- a/lib/debug/server_dap.rb +++ b/lib/debug/server_dap.rb @@ -271,6 +271,10 @@ def recv_request end end rescue RetryBecauseCantRead + # Another process consumed the message. Wait briefly for it to + # process and publish any breakpoint changes, then sync. + sleep 0.05 + @session.bp_sync_check retry end @@ -356,6 +360,7 @@ def process_request req bps << SESSION.add_line_breakpoint(path, line) end } + SESSION.bp_sync_publish send_response req, breakpoints: (bps.map do |bp| {verified: true,} end) else send_response req, breakpoints: (args['breakpoints'].map do |bp| {verified: false, message: "#{req_path} could not be located; specify source location in launch.json with \"localfsMap\" or \"localfs\""} end) @@ -391,12 +396,14 @@ def process_request req process_filter.call(bp_info['filterId'], bp_info['condition']) } + SESSION.bp_sync_publish send_response req, breakpoints: filters when 'disconnect' terminate = args.fetch("terminateDebuggee", false) SESSION.clear_all_breakpoints + SESSION.bp_sync_publish send_response req if SESSION.in_subsession? diff --git a/lib/debug/session.rb b/lib/debug/session.rb index 3a101d6ee..2cce23f59 100644 --- a/lib/debug/session.rb +++ b/lib/debug/session.rb @@ -42,6 +42,8 @@ require 'json' if ENV['RUBY_DEBUG_TEST_UI'] == 'terminal' require 'pp' +require 'set' +require 'tmpdir' class RubyVM::InstructionSequence def traceable_lines_norec lines @@ -90,10 +92,86 @@ module DEBUGGER__ class PostmortemError < RuntimeError; end + module BreakpointSync + def bp_sync_publish + return unless @process_group.multi? + @process_group.write_breakpoint_state(serialize_sync_breakpoints) + end + + def bp_sync_check + return false unless @process_group.multi? + specs = @process_group.read_breakpoint_state + return false unless specs + reconcile_breakpoints(specs) + true + end + + private + + def serialize_sync_breakpoints + @bps.filter_map { |_key, bp| bp.to_sync_data } + end + + def reconcile_breakpoints(specs) + remote_keys = Set.new + + specs.each do |spec| + key = bp_key_from_spec(spec) + next unless key + remote_keys << key + unless @bps.key?(key) + create_bp_from_spec(spec) + end + end + + @bps.delete_if do |key, bp| + if syncable_bp?(bp) && !remote_keys.include?(key) + bp.delete + true + end + end + end + + def bp_key_from_spec(spec) + case spec['type'] + when 'line' then [spec['path'], spec['line']] + when 'catch' then [:catch, spec['pat']] + when 'method' then "#{spec['klass']}#{spec['op']}#{spec['method']}" + end + end + + def create_bp_from_spec(spec) + bp = case spec['type'] + when 'line' + return unless spec['path'].is_a?(String) && spec['line'].is_a?(Integer) + LineBreakpoint.new(spec['path'], spec['line'], + cond: spec['cond'], oneshot: spec['oneshot'], + hook_call: spec['hook_call'] != false, + command: spec['command']) + when 'catch' + return unless spec['pat'].is_a?(String) + CatchBreakpoint.new(spec['pat'], + cond: spec['cond'], command: spec['command'], + path: spec['path']) + when 'method' + return unless spec['klass'].is_a?(String) && spec['op'].is_a?(String) && spec['method'].is_a?(String) + MethodBreakpoint.new(TOPLEVEL_BINDING, spec['klass'], spec['op'], spec['method'], + cond: spec['cond'], command: spec['command']) + end + + add_bp(bp) if bp + end + + def syncable_bp?(bp) + bp.syncable? + end + end + class Session attr_reader :intercepted_sigint_cmd, :process_group, :subsession_id include Color + include BreakpointSync def initialize @ui = nil @@ -1711,8 +1789,10 @@ def get_thread_client th = Thread.current DEBUGGER__.debug{ "Enter subsession (nested #{@subsession_stack.size})" } else DEBUGGER__.debug{ "Enter subsession" } + @process_group.wk_lock # blocks until no other debugger is active stop_all_threads @process_group.lock + bp_sync_check # sync breakpoints from other processes end @subsession_stack << true @@ -1724,7 +1804,11 @@ def get_thread_client th = Thread.current if @subsession_stack.empty? DEBUGGER__.debug{ "Leave subsession" } + bp_sync_publish # publish breakpoint changes to other processes @process_group.unlock + # Keep wk_lock held during step commands so the same worker + # re-enters the subsession without yielding to a sibling. + @process_group.unlock_wk_lock if type == :continue restart_all_threads else DEBUGGER__.debug{ "Leave subsession (nested #{@subsession_stack.size})" } @@ -2028,6 +2112,7 @@ def extend_feature session: nil, thread_client: nil, ui: nil class ProcessGroup def initialize @lock_file = nil + @wk_lock_file = nil end def locked? @@ -2057,10 +2142,40 @@ def multi? @lock_file end + # No-ops for single-process mode; overridden by MultiProcessGroup + def write_breakpoint_state(specs); end + def read_breakpoint_state; nil; end + + # Well-known lock for coordinating independent debugger instances + # (e.g., parallel test workers that each load the debugger independently). + # Uses process group ID so sibling processes from the same command share the lock. + # Blocks until the lock is acquired — other workers wait in line. + def wk_lock + return if multi? # MultiProcessGroup handles its own locking + ensure_wk_lock! + @wk_lock_file&.flock(File::LOCK_EX) + end + + def unlock_wk_lock + return if multi? + @wk_lock_file&.flock(File::LOCK_UN) + end + + private def ensure_wk_lock! + return if @wk_lock_file + path = File.join(Dir.tmpdir, "ruby-debug-#{Process.uid}-pgrp-#{Process.getpgrp}.lock") + @wk_lock_file = File.open(path, File::WRONLY | File::CREAT, 0600) + rescue SystemCallError => e + DEBUGGER__.warn "Failed to create well-known lock file: #{e.message}" + end + def multi_process! require 'tempfile' + require 'json' @lock_tempfile = Tempfile.open("ruby-debug-lock-") @lock_tempfile.close + @state_tempfile = Tempfile.open("ruby-debug-state-") + @state_tempfile.close extend MultiProcessGroup end end @@ -2076,6 +2191,7 @@ def after_fork child: true @lock_level = 0 @lock_file = open(@lock_tempfile.path, 'w') end + @bp_sync_version = 0 end end @@ -2146,6 +2262,34 @@ def unlock end end + def write_breakpoint_state(specs) + # Read current file version to avoid drift between processes + current_v = begin + d = JSON.parse(File.read(@state_tempfile.path)) + d['v'] + rescue + 0 + end + @bp_sync_version = [current_v, @bp_sync_version].max + 1 + data = JSON.generate({ 'v' => @bp_sync_version, 'bps' => specs }) + tmp = "#{@state_tempfile.path}.#{Process.pid}.tmp" + File.write(tmp, data, perm: 0600) + File.rename(tmp, @state_tempfile.path) + rescue SystemCallError => e + DEBUGGER__.warn "Failed to write breakpoint state: #{e.message}" + end + + def read_breakpoint_state + return nil unless File.exist?(@state_tempfile.path) + data = JSON.parse(File.read(@state_tempfile.path)) + remote_v = data['v'] + return nil if remote_v <= @bp_sync_version + @bp_sync_version = remote_v + data['bps'] + rescue JSON::ParserError, SystemCallError + nil + end + def sync &b info "sync" @@ -2547,6 +2691,7 @@ def daemon(*args) child_hook = -> { DEBUGGER__.info "Attaching after process #{parent_pid} fork to child process #{Process.pid}" SESSION.process_group.after_fork child: true + SESSION.bp_sync_check SESSION.activate on_fork: true } end diff --git a/test/console/fork_bp_sync_test.rb b/test/console/fork_bp_sync_test.rb new file mode 100644 index 000000000..002ddfb73 --- /dev/null +++ b/test/console/fork_bp_sync_test.rb @@ -0,0 +1,141 @@ +# frozen_string_literal: true + +require_relative '../support/console_test_case' + +module DEBUGGER__ + class ForkBpSyncTest < ConsoleTestCase + + # Breakpoint set in parent AFTER fork is synced to child. + # The child uses binding.break as a sync checkpoint -- when it enters + # subsession there, bp_sync_check fires and picks up the new bp. + def test_bp_set_after_fork_reaches_child + code = <<~RUBY + 1| pid = fork do + 2| sleep 2 + 3| binding.break # sync checkpoint + 4| a = 1 # bp target (set from parent after fork) + 5| end + 6| sleep 0.1 + 7| binding.break # parent stops here after fork + 8| Process.waitpid pid + RUBY + + debug_code(code) do + type 'c' + assert_line_num 7 + type 'b 4' # set bp AFTER fork + type 'c' # parent continues, bp_sync_publish writes to file + assert_line_num 3 # child at sync checkpoint + type 'c' # child continues, hits synced bp + assert_line_num 4 + type 'c' + end + end + + # Breakpoint deleted in parent after fork is removed from child. + # Child inherits bp via COW, but parent deletes it and publishes. + # When child syncs at its binding.break, the bp is reconciled away. + def test_bp_deleted_after_fork_removed_from_child + code = <<~RUBY + 1| pid = fork do + 2| sleep 2 + 3| binding.break # sync checkpoint + 4| a = 1 # bp was inherited, then deleted via sync + 5| binding.break # child should stop here instead + 6| end + 7| sleep 0.1 + 8| binding.break # parent stops here + 9| Process.waitpid pid + RUBY + + debug_code(code) do + type 'b 4' # set bp (will be inherited by child via COW) + type 'c' + assert_line_num 8 + type 'del 0' # delete bp in parent + type 'c' # parent continues, bp_sync_publish (empty set) + assert_line_num 3 # child at sync checkpoint + type 'c' # child continues, line 4 bp was removed by sync + assert_line_num 5 # child skipped line 4 + type 'c' + end + end + + # Catch breakpoint syncs to child process. + def test_catch_bp_sync_after_fork + code = <<~RUBY + 1| pid = fork do + 2| sleep 2 + 3| binding.break # sync checkpoint + 4| raise "test_error" + 5| rescue + 6| binding.break # child stops after rescue + 7| end + 8| sleep 0.5 + 9| binding.break + 10| Process.waitpid pid + RUBY + + debug_code(code) do + type 'c' + assert_line_num 9 + type 'catch RuntimeError' # set catch bp after fork + type 'c' # parent continues, publish + assert_line_num 3 # child at sync checkpoint + type 'c' + assert_line_num 4 # catch bp fires + type 'c' + assert_line_num 6 # child at rescue binding.break + type 'c' + end + end + + # Breakpoints set before fork work in child (inherited via COW + sync). + # Regression test that sync doesn't break the existing behavior. + def test_bp_before_fork_works_in_child + code = <<~RUBY + 1| pid = fork do + 2| sleep 0.5 + 3| a = 1 + 4| end + 5| Process.waitpid pid + RUBY + + debug_code(code) do + type 'b 3' + type 'c' + assert_line_num 3 + type 'c' + end + end + + # Stress test with multiple children and binding.break. + # Regression test that fork_mode: :both behavior isn't broken by sync. + def test_bp_sync_stress + code = <<~RUBY + 1| pids = 3.times.map do + 2| fork do + 3| sleep 1 + 4| 2.times do + 5| binding.break + 6| end + 7| end + 8| end + 9| sleep 0.1 + 10| binding.break + 11| pids.each { |pid| Process.waitpid pid rescue nil } + RUBY + + debug_code(code) do + type 'c' + assert_line_num 10 + type 'c' + # 3 children x 2 iterations = 6 stops at line 5 + 6.times do + assert_line_num 5 + type 'c' + end + end + end + end +end diff --git a/test/console/wk_lock_test.rb b/test/console/wk_lock_test.rb new file mode 100644 index 000000000..de2d48e11 --- /dev/null +++ b/test/console/wk_lock_test.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require_relative '../support/console_test_case' + +module DEBUGGER__ + class WellKnownLockConsoleTest < ConsoleTestCase + + # Single-process debugging is unaffected by the well-known lock. + def test_single_process_unaffected + code = <<~RUBY + 1| require "debug" + 2| DEBUGGER__.start + 3| a = 1 + 4| binding.break + 5| b = 2 + RUBY + + run_ruby(code) do + assert_line_num 3 + type 'c' + assert_line_num 4 + type 'c' + end + end + + # The well-known lock is skipped when MultiProcessGroup is active + # (fork_mode: :both via rdbg). + def test_fork_mode_both_uses_process_group_not_wk_lock + code = <<~RUBY + 1| pid = fork do + 2| sleep 0.5 + 3| a = 1 + 4| end + 5| Process.waitpid pid + RUBY + + debug_code(code) do + type 'b 3' + type 'c' + assert_line_num 3 + type 'c' + end + end + + # Independent workers that load the debugger after fork are serialized + # by the well-known lock. Each worker enters the debugger in sequence. + def test_independent_workers_serialized + code = <<~RUBY + 1| 2.times do |i| + 2| fork do + 3| require "debug" + 4| DEBUGGER__.start(nonstop: true) + 5| sleep 1 + 6| debugger + 7| $stdout.puts "worker_\#{i}_done" + 8| end + 9| end + 10| Process.waitall + RUBY + + run_ruby(code) do + assert_line_num 6 + type 'c' + assert_line_num 6 + type 'c' + end + end + end +end