Skip to content

Commit 121e4f8

Browse files
committed
Unfreeze threads for object-evaluating commands
There are several commands that evaluate objects and call methods on them, such as: - info - ls (outline) - trace object - display If the called method acquires a mutex shared with another thread (e.g. when using Timeout), then it'd cause a deadlock as all threads are stopped. For example, if there's an ActiveRecord Relation object stored as a local variable, and the user runs the `info` command, then it'd call `inspect` on it and trigger a database query, it could cause a deadlock as described in ruby#877. This commit fixes the issue by unfreezing all threads before evaluating those commands.
1 parent dea10b5 commit 121e4f8

File tree

4 files changed

+110
-14
lines changed

4 files changed

+110
-14
lines changed

lib/debug/session.rb

+17-13
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,11 @@ def request_tc(req)
256256
@tc << req
257257
end
258258

259+
def request_tc_with_freed_threads(req)
260+
restart_all_threads
261+
request_tc(req)
262+
end
263+
259264
def process_event evt
260265
# variable `@internal_info` is only used for test
261266
tc, output, ev, @internal_info, *ev_args = evt
@@ -314,7 +319,7 @@ def process_event evt
314319
if @displays.empty?
315320
wait_command_loop
316321
else
317-
request_tc [:eval, :display, @displays]
322+
request_eval :display, @displays
318323
end
319324
when :result
320325
raise "[BUG] not in subsession" if @subsession_stack.empty?
@@ -810,15 +815,15 @@ def register_default_command
810815

811816
case sub
812817
when nil
813-
request_tc [:show, :default, pat] # something useful
818+
request_tc_with_freed_threads [:show, :default, pat] # something useful
814819
when :locals
815-
request_tc [:show, :locals, pat]
820+
request_tc_with_freed_threads [:show, :locals, pat]
816821
when :ivars
817-
request_tc [:show, :ivars, pat, opt]
822+
request_tc_with_freed_threads [:show, :ivars, pat, opt]
818823
when :consts
819-
request_tc [:show, :consts, pat, opt]
824+
request_tc_with_freed_threads [:show, :consts, pat, opt]
820825
when :globals
821-
request_tc [:show, :globals, pat]
826+
request_tc_with_freed_threads [:show, :globals, pat]
822827
when :threads
823828
thread_list
824829
:retry
@@ -838,7 +843,7 @@ def register_default_command
838843
# * Show you available methods and instance variables of the given object.
839844
# * If the object is a class/module, it also lists its constants.
840845
register_command 'outline', 'o', 'ls', unsafe: false do |arg|
841-
request_tc [:show, :outline, arg]
846+
request_tc_with_freed_threads [:show, :outline, arg]
842847
end
843848

844849
# * `display`
@@ -848,9 +853,9 @@ def register_default_command
848853
register_command 'display', postmortem: false do |arg|
849854
if arg && !arg.empty?
850855
@displays << arg
851-
request_tc [:eval, :try_display, @displays]
856+
request_eval :try_display, @displays
852857
else
853-
request_tc [:eval, :display, @displays]
858+
request_eval :display, @displays
854859
end
855860
end
856861

@@ -864,7 +869,7 @@ def register_default_command
864869
if @displays[n = $1.to_i]
865870
@displays.delete_at n
866871
end
867-
request_tc [:eval, :display, @displays]
872+
request_eval :display, @displays
868873
when nil
869874
if ask "clear all?", 'N'
870875
@displays.clear
@@ -983,7 +988,7 @@ def register_default_command
983988
:retry
984989

985990
when /\Aobject\s+(.+)/
986-
request_tc [:trace, :object, $1.strip, {pattern: pattern, into: into}]
991+
request_tc_with_freed_threads [:trace, :object, $1.strip, {pattern: pattern, into: into}]
987992

988993
when /\Aoff\s+(\d+)\z/
989994
if t = @tracers.values[$1.to_i]
@@ -1165,8 +1170,7 @@ def process_command line
11651170
end
11661171

11671172
def request_eval type, src
1168-
restart_all_threads
1169-
request_tc [:eval, type, src]
1173+
request_tc_with_freed_threads [:eval, type, src]
11701174
end
11711175

11721176
def step_command type, arg

test/console/info_test.rb

+31
Original file line numberDiff line numberDiff line change
@@ -367,4 +367,35 @@ def test_ivar_abbrev
367367
end
368368
end
369369
end
370+
371+
class InfoThreadLockingTest < ConsoleTestCase
372+
def program
373+
<<~RUBY
374+
1| th0 = Thread.new{sleep}
375+
2| m = Mutex.new
376+
3| th1 = Thread.new do
377+
4| m.lock
378+
5| sleep 1
379+
6| m.unlock
380+
7| end
381+
8|
382+
9| define_method :inspect do
383+
10| m.lock
384+
11| super
385+
12| end
386+
13|
387+
14| sleep 0.5
388+
15| debugger
389+
RUBY
390+
end
391+
392+
def test_info_doesnt_cause_deadlock
393+
debug_code(program) do
394+
type 'c'
395+
type 'info'
396+
assert_line_text(/%self = main/)
397+
type 'c'
398+
end
399+
end
400+
end
370401
end

test/console/outline_test.rb

+31
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,35 @@ def test_outline_aliases
6767
end
6868
end
6969
end
70+
71+
class OutlineThreadLockingTest < ConsoleTestCase
72+
def program
73+
<<~RUBY
74+
1| th0 = Thread.new{sleep}
75+
2| m = Mutex.new
76+
3| th1 = Thread.new do
77+
4| m.lock
78+
5| sleep 1
79+
6| m.unlock
80+
7| end
81+
8|
82+
9| self.define_method :constants do # overriding constants is only one of the ways to cause deadlock with outline
83+
10| m.lock
84+
11| []
85+
12| end
86+
13|
87+
14| sleep 0.5
88+
15| debugger
89+
RUBY
90+
end
91+
92+
def test_outline_doesnt_cause_deadlock
93+
debug_code(program) do
94+
type 'c'
95+
type 'ls'
96+
assert_line_text(/locals: m/)
97+
type 'c'
98+
end
99+
end
100+
end
70101
end

test/console/trace_test.rb

+31-1
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,36 @@ def test_block_doesnt_break_tracer
473473
end
474474
end
475475

476+
class ThreadLockingTest < ConsoleTestCase
477+
def program
478+
<<~RUBY
479+
1| th0 = Thread.new{sleep}
480+
2| m = Mutex.new
481+
3| th1 = Thread.new do
482+
4| m.lock
483+
5| sleep 1
484+
6| m.unlock
485+
7| end
486+
8|
487+
9| define_singleton_method :inspect do
488+
10| m.lock
489+
11| ""
490+
12| end
491+
13|
492+
14| sleep 0.5
493+
15| debugger
494+
RUBY
495+
end
496+
497+
def test_object_tracer_doesnt_cause_deadlock
498+
debug_code(program) do
499+
type 'c'
500+
type 'trace object self'
501+
type 'c'
502+
end
503+
end
504+
end
505+
476506
class TraceCallReceiverTest < ConsoleTestCase
477507
def program
478508
<<~RUBY
@@ -544,7 +574,7 @@ def program
544574
6| p a
545575
RUBY
546576
end
547-
577+
548578
def test_1656237686
549579
debug_code(program) do
550580
type 'trace line'

0 commit comments

Comments
 (0)