ruby-changes:48660
From: ko1 <ko1@a...>
Date: Wed, 15 Nov 2017 22:21:29 +0900 (JST)
Subject: [ruby-changes:48660] ko1:r60776 (trunk): remove rb_thread_t::event_hooks.
ko1 2017-11-15 22:21:24 +0900 (Wed, 15 Nov 2017) New Revision: 60776 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=60776 Log: remove rb_thread_t::event_hooks. * vm_core.h (rb_thread_t): remove rb_thread_t::event_hooks. * vm_trace.c: all hooks are connected to vm->event_hooks and add rb_event_hook_t::filter::th to filter invoke thread. It will simplify invoking hooks code. * thread.c (thread_start_func_2): clear thread specific trace_func. * test/ruby/test_settracefunc.rb: add a test for Thread#add_trace_func. Modified files: trunk/test/ruby/test_settracefunc.rb trunk/thread.c trunk/vm.c trunk/vm_core.h trunk/vm_trace.c Index: test/ruby/test_settracefunc.rb =================================================================== --- test/ruby/test_settracefunc.rb (revision 60775) +++ test/ruby/test_settracefunc.rb (revision 60776) @@ -1772,4 +1772,75 @@ class TestSetTraceFunc < Test::Unit::Tes https://github.com/ruby/ruby/blob/trunk/test/ruby/test_settracefunc.rb#L1772 def test_trace_point_require_block assert_raise(ArgumentError) { TracePoint.new(:return) } end + + def method_for_test_thread_add_trace_func + + end + + def test_thread_add_trace_func + events = [] + base_line = __LINE__ + q = Queue.new + t = Thread.new{ + Thread.current.add_trace_func proc{|ev, file, line, *args| + events << [ev, line] + } # do not stop trace. They will be stopped at Thread termination. + q.push 1 + _x = 1 + method_for_test_thread_add_trace_func + _y = 2 + } + q.pop + method_for_test_thread_add_trace_func + t.join + assert_equal ["c-return", base_line + 3], events[0] + assert_equal ["line", base_line + 6], events[1] + assert_equal ["c-call", base_line + 6], events[2] + assert_equal ["c-return", base_line + 6], events[3] + assert_equal ["line", base_line + 7], events[4] + assert_equal ["line", base_line + 8], events[5] + assert_equal ["call", base_line + -6], events[6] + assert_equal ["return", base_line + -6], events[7] + assert_equal ["line", base_line + 9], events[8] + assert_equal nil, events[9] + + # other thread + events = [] + m2t_q = Queue.new + + t = Thread.new{ + Thread.current.abort_on_exception = true + assert_equal 1, m2t_q.pop + _x = 1 + method_for_test_thread_add_trace_func + _y = 2 + Thread.current.set_trace_func(nil) + method_for_test_thread_add_trace_func + } + # it is dirty hack. usually we shouldn't use such technique + Thread.pass until t.status == 'sleep' + + t.add_trace_func proc{|ev, file, line, *args| + if file == __FILE__ + events << [ev, line] + end + } + + method_for_test_thread_add_trace_func + + m2t_q.push 1 + t.join + + assert_equal ["c-return", base_line + 31], events[0] + assert_equal ["line", base_line + 32], events[1] + assert_equal ["line", base_line + 33], events[2] + assert_equal ["call", base_line + -6], events[3] + assert_equal ["return", base_line + -6], events[4] + assert_equal ["line", base_line + 34], events[5] + assert_equal ["line", base_line + 35], events[6] + assert_equal ["c-call", base_line + 35], events[7] # Thread.current + assert_equal ["c-return", base_line + 35], events[8] # Thread.current + assert_equal ["c-call", base_line + 35], events[9] # Thread#set_trace_func + assert_equal nil, events[10] + end end Index: vm_trace.c =================================================================== --- vm_trace.c (revision 60775) +++ vm_trace.c (revision 60776) @@ -36,6 +36,10 @@ typedef struct rb_event_hook_struct { https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L36 rb_event_hook_func_t func; VALUE data; struct rb_event_hook_struct *next; + + struct { + rb_thread_t *th; + } filter; } rb_event_hook_t; typedef void (*rb_event_hook_raw_arg_func_t)(VALUE data, const rb_trace_arg_t *arg); @@ -127,12 +131,18 @@ alloc_event_hook(rb_event_hook_func_t fu https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L131 hook->events = events; hook->func = func; hook->data = data; + + /* no filters */ + hook->filter.th = 0; + return hook; } static void -connect_event_hook(rb_hook_list_t *list, rb_event_hook_t *hook) +connect_event_hook(const rb_execution_context_t *ec, rb_event_hook_t *hook) { + rb_hook_list_t *list = &rb_ec_vm_ptr(ec)->event_hooks; + hook->next = list->hooks; list->hooks = hook; recalc_add_ruby_vm_event_flags(hook->events); @@ -140,51 +150,58 @@ connect_event_hook(rb_hook_list_t *list, https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L150 } static void -rb_threadptr_add_event_hook(rb_thread_t *th, rb_event_hook_func_t func, rb_event_flag_t events, VALUE data, rb_event_hook_flag_t hook_flags) +rb_threadptr_add_event_hook(const rb_execution_context_t *ec, rb_thread_t *th, + rb_event_hook_func_t func, rb_event_flag_t events, VALUE data, rb_event_hook_flag_t hook_flags) { rb_event_hook_t *hook = alloc_event_hook(func, events, data, hook_flags); - connect_event_hook(&th->event_hooks, hook); + hook->filter.th = th; + connect_event_hook(ec, hook); } void rb_thread_add_event_hook(VALUE thval, rb_event_hook_func_t func, rb_event_flag_t events, VALUE data) { - rb_threadptr_add_event_hook(rb_thread_ptr(thval), func, events, data, RUBY_EVENT_HOOK_FLAG_SAFE); + rb_threadptr_add_event_hook(GET_EC(), rb_thread_ptr(thval), func, events, data, RUBY_EVENT_HOOK_FLAG_SAFE); } void rb_add_event_hook(rb_event_hook_func_t func, rb_event_flag_t events, VALUE data) { rb_event_hook_t *hook = alloc_event_hook(func, events, data, RUBY_EVENT_HOOK_FLAG_SAFE); - connect_event_hook(&GET_VM()->event_hooks, hook); + connect_event_hook(GET_EC(), hook); } void rb_thread_add_event_hook2(VALUE thval, rb_event_hook_func_t func, rb_event_flag_t events, VALUE data, rb_event_hook_flag_t hook_flags) { - rb_threadptr_add_event_hook(rb_thread_ptr(thval), func, events, data, hook_flags); + rb_threadptr_add_event_hook(GET_EC(), rb_thread_ptr(thval), func, events, data, hook_flags); } void rb_add_event_hook2(rb_event_hook_func_t func, rb_event_flag_t events, VALUE data, rb_event_hook_flag_t hook_flags) { rb_event_hook_t *hook = alloc_event_hook(func, events, data, hook_flags); - connect_event_hook(&GET_VM()->event_hooks, hook); + connect_event_hook(GET_EC(), hook); } +#define MATCH_ANY_FILTER_TH ((rb_thread_t *)1) + /* if func is 0, then clear all funcs */ static int -remove_event_hook(rb_hook_list_t *list, rb_event_hook_func_t func, VALUE data) +remove_event_hook(const rb_execution_context_t *ec, const rb_thread_t *filter_th, rb_event_hook_func_t func, VALUE data) { + rb_hook_list_t *list = &rb_ec_vm_ptr(ec)->event_hooks; int ret = 0; rb_event_hook_t *hook = list->hooks; while (hook) { if (func == 0 || hook->func == func) { - if (data == Qundef || hook->data == data) { - hook->hook_flags |= RUBY_EVENT_HOOK_FLAG_DELETED; - ret+=1; - list->need_clean = TRUE; + if (hook->filter.th == filter_th || filter_th == MATCH_ANY_FILTER_TH) { + if (data == Qundef || hook->data == data) { + hook->hook_flags |= RUBY_EVENT_HOOK_FLAG_DELETED; + ret+=1; + list->need_clean = TRUE; + } } } hook = hook->next; @@ -194,45 +211,46 @@ remove_event_hook(rb_hook_list_t *list, https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L211 } static int -rb_threadptr_remove_event_hook(rb_thread_t *th, rb_event_hook_func_t func, VALUE data) +rb_threadptr_remove_event_hook(const rb_execution_context_t *ec, const rb_thread_t *filter_th, rb_event_hook_func_t func, VALUE data) { - return remove_event_hook(&th->event_hooks, func, data); + return remove_event_hook(ec, filter_th, func, data); } int rb_thread_remove_event_hook(VALUE thval, rb_event_hook_func_t func) { - return rb_threadptr_remove_event_hook(rb_thread_ptr(thval), func, Qundef); + return rb_threadptr_remove_event_hook(GET_EC(), rb_thread_ptr(thval), func, Qundef); } int rb_thread_remove_event_hook_with_data(VALUE thval, rb_event_hook_func_t func, VALUE data) { - return rb_threadptr_remove_event_hook(rb_thread_ptr(thval), func, data); + return rb_threadptr_remove_event_hook(GET_EC(), rb_thread_ptr(thval), func, data); } int rb_remove_event_hook(rb_event_hook_func_t func) { - return remove_event_hook(&GET_VM()->event_hooks, func, Qundef); + return remove_event_hook(GET_EC(), NULL, func, Qundef); } int rb_remove_event_hook_with_data(rb_event_hook_func_t func, VALUE data) { - return remove_event_hook(&GET_VM()->event_hooks, func, data); + return remove_event_hook(GET_EC(), NULL, func, data); } void rb_clear_trace_func(void) { - rb_vm_t *vm = GET_VM(); - rb_thread_t *th = 0; + rb_execution_context_t *ec = GET_EC(); + rb_threadptr_remove_event_hook(ec, MATCH_ANY_FILTER_TH, 0, Qundef); +} - list_for_each(&vm->living_threads, th, vmlt_node) { - rb_threadptr_remove_event_hook(th, 0, Qundef); - } - rb_remove_event_hook(0); +void +rb_ec_clear_current_thread_trace_func(const rb_execution_context_t *ec) +{ + rb_threadptr_remove_event_hook(ec, rb_ec_thread_ptr(ec), 0, Qundef); } /* invoke hooks */ @@ -264,7 +282,9 @@ exec_hooks_body(const rb_execution_conte https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L282 rb_event_hook_t *hook; for (hook = list->hooks; hook; hook = hook->next) { - if (!(hook->hook_flags & RUBY_EVENT_HOOK_FLAG_DELETED) && (trace_arg->event & hook->events)) { + if (!(hook->hook_flags & RUBY_EVENT_HOOK_FLAG_DELETED) && + (trace_arg->event & hook->events) && + (hook->filter.th == 0 || hook->filter.th == rb_ec_thread_ptr(ec))) { if (!(hook->hook_flags & RUBY_EVENT_HOOK_FLAG_RAW_ARG)) { (*hook->func)(trace_arg->event, hook->data, trace_arg->self, trace_arg->id, trace_arg->klass); } @@ -324,7 +344,6 @@ rb_exec_event_hooks(rb_trace_arg_t *trac https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L344 { rb_execution_context_t *ec = trace_arg->ec; rb_vm_t *vm = rb_ec_vm_ptr(ec); - rb_hook_list_t *th_event_hooks = &rb_ec_thread_ptr(ec)->event_hooks; if (trace_arg->event & RUBY_INTERNAL_EVENT_MASK) { if (ec->trace_arg && (ec->trace_arg->event & RUBY_INTERNAL_EVENT_MASK)) { @@ -334,7 +353,6 @@ rb_exec_event_hooks(rb_trace_arg_t *trac https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L353 rb_trace_arg_t *prev_trace_arg = ec->trace_arg; vm->trace_running++; ec->trace_arg = trace_arg; - exec_hooks_unprotected(ec, th_event_hooks, trace_arg); exec_hooks_unprotected(ec, &vm->event_hooks, trace_arg); ec->trace_arg = prev_trace_arg; vm->trace_running--; @@ -353,11 +371,6 @@ rb_exec_event_hooks(rb_trace_arg_t *trac https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L371 vm->trace_running++; ec->trace_arg = trace_arg; { - /* thread local traces */ - state = exec_hooks_protected(ec, th_event_hooks, trace_arg); - if (state) goto terminate; - - /* vm global traces */ state = exec_hooks_protected(ec, &vm->event_hooks, trace_arg); if (state) goto terminate; @@ -503,13 +516,13 @@ set_trace_func(VALUE obj, VALUE trace) https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L516 } static void -thread_add_trace_func(rb_thread_t *th, VALUE trace) +thread_add_trace_func(rb_execution_context_t *ec, rb_thread_t *filter_th, VALUE trace) { if (!rb_obj_is_proc(trace)) { rb_raise(rb_eTypeError, "trace_func needs to be Proc"); } - rb_threadptr_add_event_hook(th, call_trace_func, RUBY_EVENT_ALL, trace, RUBY_EVENT_HOOK_FLAG_SAFE); + rb_threadptr_add_event_hook(ec, filter_th, call_trace_func, RUBY_EVENT_ALL, trace, RUBY_EVENT_HOOK_FLAG_SAFE); } /* @@ -524,7 +537,7 @@ thread_add_trace_func(rb_thread_t *th, V https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L537 static VALUE thread_add_trace_func_m(VALUE obj, VALUE trace) { - thread_add_trace_func(rb_thread_ptr(obj), trace); + thread_add_trace_func(GET_EC(), rb_thread_ptr(obj), trace); return trace; } @@ -542,15 +555,16 @@ thread_add_trace_func_m(VALUE obj, VALUE https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L555 static VALUE thread_set_trace_func_m(VALUE target_thread, VALUE trace) { + rb_execution_context_t *ec = GET_EC(); rb_thread_t *target_th = rb_thread_ptr(target_thread); - rb_threadptr_remove_event_hook(target_th, call_trace_func, Qundef); + rb_threadptr_remove_event_hook(ec, target_th, call_trace_func, Qundef); if (NIL_P(trace)) { return Qnil; } else { - thread_add_trace_func(target_th, trace); + thread_add_trace_func(ec, target_th, trace); return trace; } } Index: thread.c =================================================================== --- thread.c (revision 60775) +++ thread.c (revision 60776) @@ -600,6 +600,8 @@ thread_do_start(rb_thread_t *th, VALUE a https://github.com/ruby/ruby/blob/trunk/thread.c#L600 } } +void rb_ec_clear_current_thread_trace_func(const rb_execution_context_t *ec); + static int thread_start_func_2(rb_thread_t *th, VALUE *stack_start, VALUE *register_stack_start) { @@ -673,6 +675,8 @@ thread_start_func_2(rb_thread_t *th, VAL https://github.com/ruby/ruby/blob/trunk/thread.c#L675 } EC_POP_TAG(); + rb_ec_clear_current_thread_trace_func(th->ec); + /* locking_mutex must be Qfalse */ if (th->locking_mutex != Qfalse) { rb_bug("thread_start_func_2: locking_mutex must not be set (%p:%"PRIxVALUE")", Index: vm_core.h =================================================================== --- vm_core.h (revision 60775) +++ vm_core.h (revision 60776) @@ -845,9 +845,6 @@ typedef struct rb_thread_struct { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L845 /* statistics data for profiler */ VALUE stat_insn_usage; - /* tracer */ - rb_hook_list_t event_hooks; - /* fiber */ rb_fiber_t *root_fiber; rb_jmpbuf_t root_jmpbuf; @@ -1723,9 +1720,9 @@ static inline void https://github.com/ruby/ruby/blob/trunk/vm_core.h#L1720 rb_exec_event_hook_orig(rb_execution_context_t *ec, const rb_event_flag_t flag, VALUE self, ID id, ID called_id, VALUE klass, VALUE data, int pop_p) { - const rb_thread_t *th = rb_ec_thread_ptr(ec); + const rb_vm_t *vm = rb_ec_vm_ptr(ec); - if ((th->event_hooks.events | th->vm->event_hooks.events) & flag) { + if (vm->event_hooks.events & flag) { struct rb_trace_arg_struct trace_arg; trace_arg.event = flag; trace_arg.ec = ec; Index: vm.c =================================================================== --- vm.c (revision 60775) +++ vm.c (revision 60776) @@ -2410,7 +2410,6 @@ rb_thread_mark(void *ptr) https://github.com/ruby/ruby/blob/trunk/vm.c#L2410 RUBY_MARK_UNLESS_NULL(th->last_status); RUBY_MARK_UNLESS_NULL(th->locking_mutex); RUBY_MARK_UNLESS_NULL(th->name); - rb_vm_trace_mark_event_hooks(&th->event_hooks); RUBY_MARK_LEAVE("thread"); } -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/