Make tracepoints with set_trace_func or TracePoint.new ractor local (ruby#15468)

Before this change, GC'ing any Ractor object caused you to lose all
enabled tracepoints across all ractors (even main). Now tracepoints are
ractor-local and this doesn't happen. Internal events are still global.

Fixes [Bug #19112]

Author: luke-gruber

depend

@@ -7606,6 +7606,7 @@ iseq.$(OBJEXT): {$(VPATH)}onigmo.h
 iseq.$(OBJEXT): {$(VPATH)}oniguruma.h
 iseq.$(OBJEXT): {$(VPATH)}prism_compile.h
 iseq.$(OBJEXT): {$(VPATH)}ractor.h
+iseq.$(OBJEXT): {$(VPATH)}ractor_core.h
 iseq.$(OBJEXT): {$(VPATH)}ruby_assert.h
 iseq.$(OBJEXT): {$(VPATH)}ruby_atomic.h
 iseq.$(OBJEXT): {$(VPATH)}rubyparser.h
@@ -19760,6 +19761,7 @@ vm_trace.$(OBJEXT): {$(VPATH)}onigmo.h
 vm_trace.$(OBJEXT): {$(VPATH)}oniguruma.h
 vm_trace.$(OBJEXT): {$(VPATH)}prism_compile.h
 vm_trace.$(OBJEXT): {$(VPATH)}ractor.h
+vm_trace.$(OBJEXT): {$(VPATH)}ractor_core.h
 vm_trace.$(OBJEXT): {$(VPATH)}ruby_assert.h
 vm_trace.$(OBJEXT): {$(VPATH)}ruby_atomic.h
 vm_trace.$(OBJEXT): {$(VPATH)}rubyparser.h

imemo.c

@@ -306,9 +306,6 @@ mark_and_move_method_entry(rb_method_entry_t *ment, bool reference_updating)
             if (!rb_gc_checking_shareable()) {
                 rb_gc_mark_and_move(&def->body.bmethod.proc);
             }
-            if (def->body.bmethod.hooks) {
-                rb_hook_list_mark_and_move(def->body.bmethod.hooks);
-            }
             break;
           case VM_METHOD_TYPE_ALIAS:
             rb_gc_mark_and_move_ptr(&def->body.alias.original_me);

iseq.c

@@ -39,6 +39,7 @@
 #include "iseq.h"
 #include "ruby/util.h"
 #include "vm_core.h"
+#include "ractor_core.h"
 #include "vm_callinfo.h"
 #include "yjit.h"
 #include "ruby/ractor.h"
@@ -161,6 +162,24 @@ iseq_clear_ic_references(const rb_iseq_t *iseq)
     }
 }
 
+
+rb_hook_list_t *
+rb_iseq_local_hooks(const rb_iseq_t *iseq, rb_ractor_t *r, bool create)
+{
+    rb_hook_list_t *hook_list = NULL;
+    st_data_t val;
+    if (st_lookup(rb_ractor_targeted_hooks(r), (st_data_t)iseq, &val)) {
+        hook_list = (rb_hook_list_t*)val;
+        RUBY_ASSERT(hook_list->type == hook_list_type_targeted_iseq);
+    }
+    else if (create) {
+        hook_list = RB_ZALLOC(rb_hook_list_t);
+        hook_list->type = hook_list_type_targeted_iseq;
+        st_insert(rb_ractor_targeted_hooks(r), (st_data_t)iseq, (st_data_t)hook_list);
+    }
+    return hook_list;
+}
+
 void
 rb_iseq_free(const rb_iseq_t *iseq)
 {
@@ -213,10 +232,6 @@ rb_iseq_free(const rb_iseq_t *iseq)
         ruby_xfree(body);
     }
 
-    if (iseq && ISEQ_EXECUTABLE_P(iseq) && iseq->aux.exec.local_hooks) {
-        rb_hook_list_free(iseq->aux.exec.local_hooks);
-    }
-
     RUBY_FREE_LEAVE("iseq");
 }
 
@@ -448,10 +463,6 @@ rb_iseq_mark_and_move(rb_iseq_t *iseq, bool reference_updating)
     else {
         /* executable */
         VM_ASSERT(ISEQ_EXECUTABLE_P(iseq));
-
-        if (iseq->aux.exec.local_hooks) {
-            rb_hook_list_mark_and_move(iseq->aux.exec.local_hooks);
-        }
     }
 
     RUBY_MARK_LEAVE("iseq");
@@ -2438,17 +2449,22 @@ rb_iseq_event_flags(const rb_iseq_t *iseq, size_t pos)
     }
 }
 
+static void rb_iseq_trace_flag_cleared(const rb_iseq_t *iseq, size_t pos);
+
 // Clear tracing event flags and turn off tracing for a given instruction as needed.
 // This is currently used after updating a one-shot line coverage for the current instruction.
 void
 rb_iseq_clear_event_flags(const rb_iseq_t *iseq, size_t pos, rb_event_flag_t reset)
 {
-    struct iseq_insn_info_entry *entry = (struct iseq_insn_info_entry *)get_insn_info(iseq, pos);
-    if (entry) {
-        entry->events &= ~reset;
-        if (!(entry->events & iseq->aux.exec.global_trace_events)) {
-            void rb_iseq_trace_flag_cleared(const rb_iseq_t *iseq, size_t pos);
-            rb_iseq_trace_flag_cleared(iseq, pos);
+    RB_VM_LOCKING() {
+        rb_vm_barrier();
+
+        struct iseq_insn_info_entry *entry = (struct iseq_insn_info_entry *)get_insn_info(iseq, pos);
+        if (entry) {
+            entry->events &= ~reset;
+            if (!(entry->events & iseq->aux.exec.global_trace_events)) {
+                rb_iseq_trace_flag_cleared(iseq, pos);
+            }
         }
     }
 }
@@ -3930,14 +3946,15 @@ rb_vm_insn_decode(const VALUE encoded)
 
 // Turn on or off tracing for a given instruction address
 static inline int
-encoded_iseq_trace_instrument(VALUE *iseq_encoded_insn, rb_event_flag_t turnon, bool remain_current_trace)
+encoded_iseq_trace_instrument(VALUE *iseq_encoded_insn, rb_event_flag_t turnon, bool remain_traced)
 {
+    ASSERT_vm_locking();
     st_data_t key = (st_data_t)*iseq_encoded_insn;
     st_data_t val;
 
     if (st_lookup(encoded_insn_data, key, &val)) {
         insn_data_t *e = (insn_data_t *)val;
-        if (remain_current_trace && key == (st_data_t)e->trace_encoded_insn) {
+        if (remain_traced && key == (st_data_t)e->trace_encoded_insn) {
             turnon = 1;
         }
         *iseq_encoded_insn = (VALUE) (turnon ? e->trace_encoded_insn : e->notrace_encoded_insn);
@@ -3948,7 +3965,7 @@ encoded_iseq_trace_instrument(VALUE *iseq_encoded_insn, rb_event_flag_t turnon,
 }
 
 // Turn off tracing for an instruction at pos after tracing event flags are cleared
-void
+static void
 rb_iseq_trace_flag_cleared(const rb_iseq_t *iseq, size_t pos)
 {
     const struct rb_iseq_constant_body *const body = ISEQ_BODY(iseq);
@@ -3974,14 +3991,16 @@ add_bmethod_events(rb_event_flag_t events)
 
 // Note, to support call/return events for bmethods, turnon_event can have more events than tpval.
 static int
-iseq_add_local_tracepoint(const rb_iseq_t *iseq, rb_event_flag_t turnon_events, VALUE tpval, unsigned int target_line)
+iseq_add_local_tracepoint(const rb_iseq_t *iseq, rb_event_flag_t turnon_events, VALUE tpval, unsigned int target_line, rb_ractor_t *r)
 {
     unsigned int pc;
     int n = 0;
     const struct rb_iseq_constant_body *const body = ISEQ_BODY(iseq);
     VALUE *iseq_encoded = (VALUE *)body->iseq_encoded;
+    rb_iseq_t *iseq_mut = (rb_iseq_t*)iseq;
 
     VM_ASSERT(ISEQ_EXECUTABLE_P(iseq));
+    ASSERT_vm_locking_with_barrier();
 
     for (pc=0; pc<body->iseq_size;) {
         const struct iseq_insn_info_entry *entry = get_insn_info(iseq, pc);
@@ -4003,11 +4022,9 @@ iseq_add_local_tracepoint(const rb_iseq_t *iseq, rb_event_flag_t turnon_events,
     }
 
     if (n > 0) {
-        if (iseq->aux.exec.local_hooks == NULL) {
-            ((rb_iseq_t *)iseq)->aux.exec.local_hooks = RB_ZALLOC(rb_hook_list_t);
-            iseq->aux.exec.local_hooks->is_local = true;
-        }
-        rb_hook_list_connect_tracepoint((VALUE)iseq, iseq->aux.exec.local_hooks, tpval, target_line);
+        rb_hook_list_t *hook_list = rb_iseq_local_hooks(iseq, r, true);
+        rb_hook_list_connect_local_tracepoint(hook_list, tpval, target_line);
+        iseq_mut->aux.exec.local_hooks_cnt++;
     }
 
     return n;
@@ -4018,19 +4035,21 @@ struct trace_set_local_events_struct {
     VALUE tpval;
     unsigned int target_line;
     int n;
+    rb_ractor_t *r;
 };
 
 static void
 iseq_add_local_tracepoint_i(const rb_iseq_t *iseq, void *p)
 {
     struct trace_set_local_events_struct *data = (struct trace_set_local_events_struct *)p;
-    data->n += iseq_add_local_tracepoint(iseq, data->turnon_events, data->tpval, data->target_line);
+    data->n += iseq_add_local_tracepoint(iseq, data->turnon_events, data->tpval, data->target_line, data->r);
     iseq_iterate_children(iseq, iseq_add_local_tracepoint_i, p);
 }
 
 int
 rb_iseq_add_local_tracepoint_recursively(const rb_iseq_t *iseq, rb_event_flag_t turnon_events, VALUE tpval, unsigned int target_line, bool target_bmethod)
 {
+    ASSERT_vm_locking_with_barrier();
     struct trace_set_local_events_struct data;
     if (target_bmethod) {
         turnon_events = add_bmethod_events(turnon_events);
@@ -4039,35 +4058,52 @@ rb_iseq_add_local_tracepoint_recursively(const rb_iseq_t *iseq, rb_event_flag_t
     data.tpval = tpval;
     data.target_line = target_line;
     data.n = 0;
+    data.r = GET_RACTOR();
 
     iseq_add_local_tracepoint_i(iseq, (void *)&data);
-    if (0) rb_funcall(Qnil, rb_intern("puts"), 1, rb_iseq_disasm(iseq)); /* for debug */
+    if (0) fprintf(stderr, "Iseq disasm:\n:%s", RSTRING_PTR(rb_iseq_disasm(iseq))); /* for debug */
     return data.n;
 }
 
 static int
-iseq_remove_local_tracepoint(const rb_iseq_t *iseq, VALUE tpval)
+iseq_remove_local_tracepoint(const rb_iseq_t *iseq, VALUE tpval, rb_ractor_t *r)
 {
     int n = 0;
+    unsigned int num_hooks_left;
+    unsigned int pc;
+    const struct rb_iseq_constant_body *body;
+    rb_iseq_t *iseq_mut = (rb_iseq_t*)iseq;
+    rb_hook_list_t *hook_list;
+    VALUE *iseq_encoded;
+    ASSERT_vm_locking_with_barrier();
 
-    if (iseq->aux.exec.local_hooks) {
-        unsigned int pc;
-        const struct rb_iseq_constant_body *const body = ISEQ_BODY(iseq);
-        VALUE *iseq_encoded = (VALUE *)body->iseq_encoded;
+    hook_list = rb_iseq_local_hooks(iseq, r, false);
+
+    if (hook_list) {
         rb_event_flag_t local_events = 0;
 
-        rb_hook_list_remove_tracepoint(iseq->aux.exec.local_hooks, tpval);
-        local_events = iseq->aux.exec.local_hooks->events;
+        rb_event_flag_t prev_events = hook_list->events;
+        if (rb_hook_list_remove_local_tracepoint(hook_list, tpval)) {
+            RUBY_ASSERT(iseq->aux.exec.local_hooks_cnt > 0);
+            iseq_mut->aux.exec.local_hooks_cnt--;
+            local_events = hook_list->events; // remaining events for this ractor
+            num_hooks_left = rb_hook_list_count(hook_list);
+            if (local_events == 0 && prev_events != 0) {
+                st_delete(rb_ractor_targeted_hooks(r), (st_data_t*)&iseq, NULL);
+                rb_hook_list_free(hook_list);
+            }
 
-        if (local_events == 0) {
-            rb_hook_list_free(iseq->aux.exec.local_hooks);
-            ((rb_iseq_t *)iseq)->aux.exec.local_hooks = NULL;
-        }
+            if (iseq->aux.exec.local_hooks_cnt == num_hooks_left) {
+                body = ISEQ_BODY(iseq);
+                iseq_encoded = (VALUE *)body->iseq_encoded;
+                local_events = add_bmethod_events(local_events);
+                for (pc = 0; pc<body->iseq_size;) {
+                    rb_event_flag_t pc_events = rb_iseq_event_flags(iseq, pc);
+                    pc += encoded_iseq_trace_instrument(&iseq_encoded[pc], pc_events & (local_events | iseq->aux.exec.global_trace_events), false);
+                }
+            }
 
-        local_events = add_bmethod_events(local_events);
-        for (pc = 0; pc<body->iseq_size;) {
-            rb_event_flag_t pc_events = rb_iseq_event_flags(iseq, pc);
-            pc += encoded_iseq_trace_instrument(&iseq_encoded[pc], pc_events & (local_events | iseq->aux.exec.global_trace_events), false);
+            n++;
         }
     }
     return n;
@@ -4076,22 +4112,25 @@ iseq_remove_local_tracepoint(const rb_iseq_t *iseq, VALUE tpval)
 struct trace_clear_local_events_struct {
     VALUE tpval;
     int n;
+    rb_ractor_t *r;
 };
 
 static void
 iseq_remove_local_tracepoint_i(const rb_iseq_t *iseq, void *p)
 {
     struct trace_clear_local_events_struct *data = (struct trace_clear_local_events_struct *)p;
-    data->n += iseq_remove_local_tracepoint(iseq, data->tpval);
+    data->n += iseq_remove_local_tracepoint(iseq, data->tpval, data->r);
     iseq_iterate_children(iseq, iseq_remove_local_tracepoint_i, p);
 }
 
 int
-rb_iseq_remove_local_tracepoint_recursively(const rb_iseq_t *iseq, VALUE tpval)
+rb_iseq_remove_local_tracepoint_recursively(const rb_iseq_t *iseq, VALUE tpval, rb_ractor_t *r)
 {
     struct trace_clear_local_events_struct data;
+    ASSERT_vm_locking_with_barrier();
     data.tpval = tpval;
     data.n = 0;
+    data.r = r;
 
     iseq_remove_local_tracepoint_i(iseq, (void *)&data);
     return data.n;
@@ -4109,11 +4148,14 @@ rb_iseq_trace_set(const rb_iseq_t *iseq, rb_event_flag_t turnon_events)
         return;
     }
     else {
+        // NOTE: this does not need VM barrier if it's a new ISEQ
         unsigned int pc;
         const struct rb_iseq_constant_body *const body = ISEQ_BODY(iseq);
+
         VALUE *iseq_encoded = (VALUE *)body->iseq_encoded;
         rb_event_flag_t enabled_events;
-        rb_event_flag_t local_events = iseq->aux.exec.local_hooks ? iseq->aux.exec.local_hooks->events : 0;
+        rb_hook_list_t *local_hooks = rb_iseq_local_hooks(iseq, GET_RACTOR(), false);
+        rb_event_flag_t local_events = local_hooks ? local_hooks->events : 0;
         ((rb_iseq_t *)iseq)->aux.exec.global_trace_events = turnon_events;
         enabled_events = add_bmethod_events(turnon_events | local_events);
 
@@ -4129,6 +4171,7 @@ void rb_vm_cc_general(const struct rb_callcache *cc);
 static bool
 clear_attr_cc(VALUE v)
 {
+    ASSERT_vm_locking_with_barrier();
     if (imemo_type_p(v, imemo_callcache) && vm_cc_ivar_p((const struct rb_callcache *)v)) {
         rb_vm_cc_general((struct rb_callcache *)v);
         return true;
@@ -4141,6 +4184,7 @@ clear_attr_cc(VALUE v)
 static bool
 clear_bf_cc(VALUE v)
 {
+    ASSERT_vm_locking_with_barrier();
     if (imemo_type_p(v, imemo_callcache) && vm_cc_bf_p((const struct rb_callcache *)v)) {
         rb_vm_cc_general((struct rb_callcache *)v);
         return true;
@@ -4166,7 +4210,10 @@ clear_attr_ccs_i(void *vstart, void *vend, size_t stride, void *data)
 void
 rb_clear_attr_ccs(void)
 {
-    rb_objspace_each_objects(clear_attr_ccs_i, NULL);
+    RB_VM_LOCKING() {
+        rb_vm_barrier();
+        rb_objspace_each_objects(clear_attr_ccs_i, NULL);
+    }
 }
 
 static int
@@ -4185,6 +4232,7 @@ clear_bf_ccs_i(void *vstart, void *vend, size_t stride, void *data)
 void
 rb_clear_bf_ccs(void)
 {
+    ASSERT_vm_locking_with_barrier();
     rb_objspace_each_objects(clear_bf_ccs_i, NULL);
 }
 
@@ -4214,7 +4262,10 @@ trace_set_i(void *vstart, void *vend, size_t stride, void *data)
 void
 rb_iseq_trace_set_all(rb_event_flag_t turnon_events)
 {
-    rb_objspace_each_objects(trace_set_i, &turnon_events);
+    RB_VM_LOCKING() {
+        rb_vm_barrier();
+        rb_objspace_each_objects(trace_set_i, &turnon_events);
+    }
 }
 
 VALUE

iseq.h

@@ -190,10 +190,12 @@ const rb_iseq_t *rb_iseq_ibf_load_bytes(const char *cstr, size_t);
 VALUE rb_iseq_ibf_load_extra_data(VALUE str);
 void rb_iseq_init_trace(rb_iseq_t *iseq);
 int rb_iseq_add_local_tracepoint_recursively(const rb_iseq_t *iseq, rb_event_flag_t turnon_events, VALUE tpval, unsigned int target_line, bool target_bmethod);
-int rb_iseq_remove_local_tracepoint_recursively(const rb_iseq_t *iseq, VALUE tpval);
+int rb_iseq_remove_local_tracepoint_recursively(const rb_iseq_t *iseq, VALUE tpval, rb_ractor_t *r);
 const rb_iseq_t *rb_iseq_load_iseq(VALUE fname);
 const rb_iseq_t *rb_iseq_compile_iseq(VALUE str, VALUE fname);
 int rb_iseq_opt_frozen_string_literal(void);
+rb_hook_list_t *rb_iseq_local_hooks(const rb_iseq_t *iseq, rb_ractor_t *r, bool create);
+
 
 #if VM_INSN_INFO_TABLE_IMPL == 2
 unsigned int *rb_iseq_insns_info_decode_positions(const struct rb_iseq_constant_body *body);

method.h

@@ -166,8 +166,8 @@ typedef struct rb_method_refined_struct {
 
 typedef struct rb_method_bmethod_struct {
     VALUE proc; /* should be marked */
-    struct rb_hook_list_struct *hooks;
     rb_serial_t defined_ractor_id;
+    unsigned int local_hooks_cnt;
 } rb_method_bmethod_t;
 
 enum method_optimized_type {
@@ -208,6 +208,8 @@ struct rb_method_definition_struct {
 };
 
 struct rb_id_table;
+struct rb_ractor_struct;
+struct rb_hook_list_struct;
 
 typedef struct rb_method_definition_struct rb_method_definition_t;
 STATIC_ASSERT(sizeof_method_def, offsetof(rb_method_definition_t, body) <= 8);
@@ -267,5 +269,8 @@ void rb_vm_delete_cc_refinement(const struct rb_callcache *cc);
 void rb_clear_method_cache(VALUE klass_or_module, ID mid);
 void rb_clear_all_refinement_method_cache(void);
 void rb_invalidate_method_caches(struct rb_id_table *cm_tbl, VALUE cc_tbl);
+struct rb_hook_list_struct *rb_method_def_local_hooks(rb_method_definition_t *def, struct rb_ractor_struct *cr, bool create);
+void rb_method_definition_addref(rb_method_definition_t *def);
+void rb_method_definition_release(rb_method_definition_t *def);
 
 #endif /* RUBY_METHOD_H */