Feature #15473
closedTracePoint#enable(target_thread:) to specify a Thread
Description
We introduced TracePoint#enable(target:, target_line:)
keyword.
How about to introduce target_thread:
keyword which specify enabling threads?
TracePoint.new(:line){|tp| p tp}.enable(target_thread: Thread.current) do
p 1
Thread.new{
p 10
p 12
}.join
p 2
end
#<TracePoint:line@/home/ko1/src/ruby/trunk/test.rb:3>
1
#<TracePoint:line@/home/ko1/src/ruby/trunk/test.rb:4>
10
12
#<TracePoint:line@/home/ko1/src/ruby/trunk/test.rb:8>
2
Maybe Thread.current
is frequent pattern, so accepting :current` symbol is reasonable for me.
TracePoint.new(:line){|tp| p tp}.enable(target_thread: :current) do
p 1
Thread.new{
p 10
p 12
}.join
p 2
end
# same result
This is a patch.
Index: prelude.rb
===================================================================
--- prelude.rb (revision 66594)
+++ prelude.rb (working copy)
@@ -133,8 +133,8 @@ class IO
end
class TracePoint
- def enable target: nil, target_line: nil, &blk
- self.__enable target, target_line, &blk
+ def enable target: nil, target_line: nil, target_thread: nil, &blk
+ self.__enable target, target_line, target_thread, &blk
end
end
Index: vm_trace.c
===================================================================
--- vm_trace.c (revision 66594)
+++ vm_trace.c (working copy)
@@ -1398,11 +1398,26 @@ rb_hook_list_remove_tracepoint(rb_hook_l
*
*/
static VALUE
-tracepoint_enable_m(VALUE tpval, VALUE target, VALUE target_line)
+tracepoint_enable_m(VALUE tpval, VALUE target, VALUE target_line, VALUE target_thread)
{
rb_tp_t *tp = tpptr(tpval);
int previous_tracing = tp->tracing;
+ if (RTEST(target_thread)) {
+ static VALUE sym_current = 0;
+ if (sym_current == 0) sym_current = rb_id2sym(rb_intern("current"));
+
+ if (target_thread == sym_current) {
+ tp->target_th = GET_THREAD();
+ }
+ else {
+ tp->target_th = rb_thread_ptr(target_thread);
+ }
+ }
+ else {
+ tp->target_th = NULL;
+ }
+
if (NIL_P(target)) {
if (!NIL_P(target_line)) {
rb_raise(rb_eArgError, "only target_line is specified");
@@ -1801,7 +1816,7 @@ Init_vm_trace(void)
*/
rb_define_singleton_method(rb_cTracePoint, "trace", tracepoint_trace_s, -1);
- rb_define_method(rb_cTracePoint, "__enable", tracepoint_enable_m, 2);
+ rb_define_method(rb_cTracePoint, "__enable", tracepoint_enable_m, 3);
rb_define_method(rb_cTracePoint, "disable", tracepoint_disable_m, 0);
rb_define_method(rb_cTracePoint, "enabled?", rb_tracepoint_enabled_p, 0);
This is a similar feature proposed at https://bugs.ruby-lang.org/issues/13483 but no compatibility issue.
Updated by shevegen (Robert A. Heiler) over 5 years ago
To me personally the proposal seems fine. I like the shortcut :current for Thread.current
which makes sense (to me), as Thread.current already exists as name, so it appears intuitive
to me.
For sake of completion, I would recommend showing how/if we can target other threads too,
and how it would look like. Unless I overlooked something, I believe we have only examples
for Thread.current / :current so far.
Updated by Eregon (Benoit Daloze) over 5 years ago
ko1 (Koichi Sasada) wrote:
Maybe
Thread.current
is frequent pattern, so accepting :current` symbol is reasonable for me.
I think it's not needed, and that passing Thread.current
is clearer and simpler, so I would propose to not add this special shortcut.
The feature seems useful and good to me, and clearer (more explicit & compatible) than #13483.
BTW, I noticed ri TracePoint#enable
doesn't show any documentation (because it's attached as __enable
), and that target: and target_line: are not documented.
Updated by ko1 (Koichi Sasada) over 5 years ago
Thank you for your comments.
For sake of completion, I would recommend showing how/if we can target other threads too, and how it would look like.
I don't have good example, but there is a Thread#set_trace_func
to specify the thread.
For example, a debugger running on one dedicated thread and it monitors a thread?
Eregon (Benoit Daloze) wrote:
I think it's not needed, and that passing
Thread.current
is clearer and simpler, so I would propose to not add this special shortcut.
I can agree it. Or introduce other keywords like current_thread: true
(maybe not a good name).
Anyway, I'll introduce target_thraed:
soon without :current
shortcut if there are no other objections.
BTW, I noticed
ri TracePoint#enable
doesn't show any documentation (because it's attached as__enable
), and that target: and target_line: are not documented.
:(
Updated by ko1 (Koichi Sasada) over 5 years ago
- Status changed from Open to Closed