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.


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@60776 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

test/ruby/test_settracefunc.rb

@@ -1772,4 +1772,75 @@ class TestSetTraceFunc < Test::Unit::TestCase
   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

thread.c

@@ -600,6 +600,8 @@ thread_do_start(rb_thread_t *th, VALUE args)
     }
 }
 
+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, VALUE *stack_start, VALUE *register_stack_s
 	}
 	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")",

vm.c

@@ -2410,7 +2410,6 @@ rb_thread_mark(void *ptr)
     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");
 }

vm_core.h

@@ -845,9 +845,6 @@ typedef struct rb_thread_struct {
     /* 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
 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;

vm_trace.c

@@ -36,6 +36,10 @@ typedef struct rb_event_hook_struct {
     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 func, rb_event_flag_t events, VALUE data,
     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, rb_event_hook_t *hook)
 }
 
 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) {
+	    if (hook->filter.th == filter_th || filter_th == MATCH_ANY_FILTER_TH) {
-		hook->hook_flags |= RUBY_EVENT_HOOK_FLAG_DELETED;
+		if (data == Qundef || hook->data == data) {
-		ret+=1;
+		    hook->hook_flags |= RUBY_EVENT_HOOK_FLAG_DELETED;
-		list->need_clean = TRUE;
+		    ret+=1;
+		    list->need_clean = TRUE;
+		}
 	    }
 	}
 	hook = hook->next;
@@ -194,45 +211,46 @@ remove_event_hook(rb_hook_list_t *list, rb_event_hook_func_t func, VALUE data)
 }
 
 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_execution_context_t *ec = GET_EC();
-    rb_thread_t *th = 0;
+    rb_threadptr_remove_event_hook(ec, MATCH_ANY_FILTER_TH, 0, Qundef);
+}
 
-    list_for_each(&vm->living_threads, th, vmlt_node) {
+void
-	rb_threadptr_remove_event_hook(th, 0, Qundef);
+rb_ec_clear_current_thread_trace_func(const rb_execution_context_t *ec)
-    }
+{
-    rb_remove_event_hook(0);
+    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_context_t *ec, rb_hook_list_t *list, const rb
     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 *trace_arg, int pop_p)
 {
     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 *trace_arg, int pop_p)
 	    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 *trace_arg, int pop_p)
 	    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)
 }
 
 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, VALUE trace)
 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 trace)
 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;
     }
 }