Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
86 changes: 50 additions & 36 deletions bin/gdb_wrapper
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env ruby

require 'optparse'
require 'thread'
require 'ostruct'

$stdout.sync = true
Expand Down Expand Up @@ -78,51 +79,64 @@ require 'ruby-debug-ide/attach/util'
require 'ruby-debug-ide/attach/native_debugger'
require 'ruby-debug-ide/attach/process_thread'

debugger = choose_debugger(options.ruby_path, options.pid, options.gems_to_include, debugger_loader_path, argv)
pids = get_child_pids(options.pid.to_s)
attach_threads = Array.new

trap('INT') do
unless debugger.exited?
$stderr.puts "backtraces for threads:\n\n"
process_threads = debugger.process_threads
if process_threads
process_threads.each do |thread|
$stderr.puts "#{thread.thread_info}\n#{thread.last_bt}\n\n"
end
end
debugger.exit
pids.each_with_index do |pid, i|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of nested methods deserve method extraction. Please split the code into sensible methods


if(i == 1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the extraction of the logic below into methods this each_with_index and if i == ..., which are not beautiful, could be transformed to normal logic like "do something with the parent process and do something else with all its children" without any additional overhead.

argv = reset_port(ARGV) # Only the first process works with the initialized port, the rest need to be recalculated
end
exit!
end

debugger.attach_to_process
debugger.set_flags
attach_threads << Thread.new(argv) do |argv|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another possible way to do this is to replace each with map and then just chain call the result.


if options.uid
DebugPrinter.print_debug("changing current uid from #{Process.uid} to #{options.uid}")
Process::Sys.setuid(options.uid.to_i)
end
debugger = choose_debugger(options.ruby_path, options.gems_to_include, debugger_loader_path, argv)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of removing pid as a instance variable of a NativeDebugger? Also, does it work at all if one attaches from multiple processes to one ruby executable?


if debugger.check_already_under_debug
$stderr.puts "Process #{debugger.pid} is already under debug"
debugger.exit
exit!
end
trap('INT') do
unless debugger.exited?
$stderr.puts "backtraces for threads:\n\n"
process_threads = debugger.process_threads
if process_threads
process_threads.each do |thread|
$stderr.puts "#{thread.thread_info}\n#{thread.last_bt}\n\n"
end
end
debugger.exit
end
exit!
end

should_check_threads_state = true
debugger.attach_to_process(pid)
debugger.set_flags

while should_check_threads_state
should_check_threads_state = false
debugger.update_threads.each do |thread|
thread.switch
while thread.need_finish_frame
should_check_threads_state = true
thread.finish
if debugger.check_already_under_debug
$stderr.puts "Process #{debugger.pid} is already under debug"
debugger.exit
exit!
end

should_check_threads_state = true

while should_check_threads_state
should_check_threads_state = false
debugger.update_threads.each do |thread|
thread.switch
while thread.need_finish_frame
should_check_threads_state = true
thread.finish
end
end
end

debugger.wait_line_event
debugger.load_debugger
debugger.exit
end
end

debugger.wait_line_event
debugger.load_debugger
debugger.exit

attach_threads.each {|thread| thread.join}
if options.uid
DebugPrinter.print_debug("changing current uid from #{Process.uid} to #{options.uid}")
Process::Sys.setuid(options.uid.to_i)
end
sleep
2 changes: 1 addition & 1 deletion bin/rdebug-ide
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ EOB
opts.separator ""
opts.separator "Options:"
opts.on("-h", "--host HOST", "Host name used for remote debugging") {|host| options.host = host}
opts.on("-p", "--port PORT", Integer, "Port used for remote debugging") {|port| options.port = port}
opts.on("-p", "--port PORT", Integer, "Port used for remote debugging") {|port| options.port = port}
opts.on("--dispatcher-port PORT", Integer, "Port used for multi-process debugging dispatcher") do |dp|
options.dispatcher_port = dp
end
Expand Down
4 changes: 2 additions & 2 deletions lib/ruby-debug-ide/attach/gdb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

class GDB < NativeDebugger

def initialize(executable, pid, flags, gems_to_include, debugger_loader_path, argv)
super(executable, pid, flags, gems_to_include, debugger_loader_path, argv)
def initialize(executable, flags, gems_to_include, debugger_loader_path, argv)
super(executable, flags, gems_to_include, debugger_loader_path, argv)
end

def set_flags
Expand Down
4 changes: 2 additions & 2 deletions lib/ruby-debug-ide/attach/lldb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

class LLDB < NativeDebugger

def initialize(executable, pid, flags, gems_to_include, debugger_loader_path, argv)
super(executable, pid, flags, gems_to_include, debugger_loader_path, argv)
def initialize(executable, flags, gems_to_include, debugger_loader_path, argv)
super(executable, flags, gems_to_include, debugger_loader_path, argv)
end

def set_flags
Expand Down
8 changes: 3 additions & 5 deletions lib/ruby-debug-ide/attach/native_debugger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@ class NativeDebugger
attr_reader :pid, :main_thread, :process_threads, :pipe

# @param executable -- path to ruby interpreter
# @param pid -- pid of process you want to debug
# @param flags -- flags you want to specify to your debugger as a string (e.g. "-nx -nh" for gdb to disable .gdbinit)
def initialize(executable, pid, flags, gems_to_include, debugger_loader_path, argv)
@pid = pid
def initialize(executable, flags, gems_to_include, debugger_loader_path, argv)
@delimiter = '__OUTPUT_FINISHED__' # for getting response
@tbreak = '__func_to_set_breakpoint_at'
@main_thread = nil
Expand Down Expand Up @@ -40,8 +38,8 @@ def find_attach_lib(debase_path)
raise 'Could not find attach library'
end

def attach_to_process
execute "attach #{@pid}"
def attach_to_process(pid)
execute "attach #{pid}"
end

def execute(command)
Expand Down
39 changes: 35 additions & 4 deletions lib/ruby-debug-ide/attach/util.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,36 @@
require 'ruby-debug-ide/attach/lldb'
require 'ruby-debug-ide/attach/gdb'
require 'socket'

def get_child_pids(pid)
pids = Array.new

q = Queue.new
q.push(pid)

while(!q.empty?) do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spacing (here and after)

pid = q.pop
pids << pid

if(command_exists 'pgrep')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're checking for the command presence each time, does it mean that the command may disappear during children collection? If not, process such check beforehand and, ideally, also incapsulate running external processes into a separate method(s) in order to fix possible related bugs in a dedicated place.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the command does not exist, then only one element will be added to the queue and the check will be performed once. But it does not look very nice, I agree

pipe = IO.popen("pgrep -P #{pid}")

pipe.readlines.each do |child_pid|
q.push(child_pid.to_i)
end
end
end

pids
end

def reset_port(argv)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't get the purpose of that method. I feel uncomfortable with re-checking argv values once again when you have an appropriate library for that.
And also, mutating the parameter container is a bad practice.

Copy link
Copy Markdown
Collaborator Author

@viuginick1 viuginick1 Jul 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case argv is transferred to rdebug as a string in load_debugger method

argv.each_with_index do |val, i|
argv[i + 1] = -1 if(val == '--port')
end

'["' + argv * '", "' + '"]'
end

def command_exists(command)
checking_command = "checking command #{command} for existence\n"
Expand All @@ -12,14 +43,14 @@ def command_exists(command)
$?.exitstatus == 0
end

def choose_debugger(ruby_path, pid, gems_to_include, debugger_loader_path, argv)
def choose_debugger(ruby_path, gems_to_include, debugger_loader_path, argv)
if command_exists(LLDB.to_s)
debugger = LLDB.new(ruby_path, pid, '--no-lldbinit', gems_to_include, debugger_loader_path, argv)
debugger = LLDB.new(ruby_path, '--no-lldbinit', gems_to_include, debugger_loader_path, argv)
elsif command_exists(GDB.to_s)
debugger = GDB.new(ruby_path, pid, '-nh -nx', gems_to_include, debugger_loader_path, argv)
debugger = GDB.new(ruby_path, '-nh -nx', gems_to_include, debugger_loader_path, argv)
else
raise 'Neither gdb nor lldb was found. Aborting.'
end

debugger
end
end
6 changes: 5 additions & 1 deletion lib/ruby-debug-ide/multiprocess/pre_child.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ def pre_child(options = nil)
'notify_dispatcher' => true
)

if(options.port == -1)
options.port = find_free_port(options.host)
options.notify_dispatcher = true
end

start_debugger(options)
end

Expand All @@ -39,7 +44,6 @@ def start_debugger(options)
Debugger.keep_frame_binding = options.frame_bind
Debugger.tracing = options.tracing
Debugger.cli_debug = options.cli_debug

Debugger.prepare_debugger(options)
end

Expand Down
2 changes: 1 addition & 1 deletion lib/ruby-debug-ide/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module Debugger
IDE_VERSION='0.6.1.beta3'
IDE_VERSION='0.6.1.beta5'
end