ruby-changes:64421
From: Koichi <ko1@a...>
Date: Tue, 22 Dec 2020 00:03:16 +0900 (JST)
Subject: [ruby-changes:64421] a2950369bd (master): TracePoint.new(&block) should be ractor-local
https://git.ruby-lang.org/ruby.git/commit/?id=a2950369bd From a2950369bd8a5866092f6badf59b0811653a6092 Mon Sep 17 00:00:00 2001 From: Koichi Sasada <ko1@a...> Date: Sat, 19 Dec 2020 06:38:58 +0900 Subject: TracePoint.new(&block) should be ractor-local TracePoint should be ractor-local because the Proc can violate the Ractor-safe. diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 797ad0a..a853e91 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -1199,6 +1199,19 @@ assert_equal '[false, false, true, true]', %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_ractor.rb#L1199 r } +# TracePoint with normal Proc should be Ractor local +assert_equal '[4, 8]', %q{ + rs = [] + TracePoint.new(:line){|tp| rs << tp.lineno if tp.path == __FILE__}.enable do + Ractor.new{ # line 4 + a = 1 + b = 2 + }.take + c = 3 # line 8 + end + rs +} + # Ractor deep copies frozen objects (ary) assert_equal '[true, false]', %q{ Ractor.new([[]].freeze) { |ary| diff --git a/common.mk b/common.mk index 2cfa952..ca237d7 100644 --- a/common.mk +++ b/common.mk @@ -16316,6 +16316,7 @@ vm_trace.$(OBJEXT): {$(VPATH)}mjit.h https://github.com/ruby/ruby/blob/trunk/common.mk#L16316 vm_trace.$(OBJEXT): {$(VPATH)}node.h vm_trace.$(OBJEXT): {$(VPATH)}onigmo.h vm_trace.$(OBJEXT): {$(VPATH)}oniguruma.h +vm_trace.$(OBJEXT): {$(VPATH)}ractor.h vm_trace.$(OBJEXT): {$(VPATH)}ruby_assert.h vm_trace.$(OBJEXT): {$(VPATH)}ruby_atomic.h vm_trace.$(OBJEXT): {$(VPATH)}st.h diff --git a/ractor.c b/ractor.c index b856419..f535e6e 100644 --- a/ractor.c +++ b/ractor.c @@ -197,6 +197,7 @@ ractor_mark(void *ptr) https://github.com/ruby/ruby/blob/trunk/ractor.c#L197 rb_gc_mark(r->r_stdin); rb_gc_mark(r->r_stdout); rb_gc_mark(r->r_stderr); + rb_hook_list_mark(&r->event_hooks); if (r->threads.cnt > 0) { rb_thread_t *th = 0; @@ -230,6 +231,7 @@ ractor_free(void *ptr) https://github.com/ruby/ruby/blob/trunk/ractor.c#L231 ractor_queue_free(&r->sync.incoming_queue); ractor_waiting_list_free(&r->sync.taking_ractors); ractor_local_storage_free(r); + rb_hook_list_free(&r->event_hooks); ruby_xfree(r); } @@ -2126,6 +2128,12 @@ rb_ractor_stderr_set(VALUE err) https://github.com/ruby/ruby/blob/trunk/ractor.c#L2128 } } +rb_hook_list_t * +rb_ractor_hooks(rb_ractor_t *cr) +{ + return &cr->event_hooks; +} + /// traverse function // 2: stop search diff --git a/ractor_core.h b/ractor_core.h index daa652e..88b1126 100644 --- a/ractor_core.h +++ b/ractor_core.h @@ -138,6 +138,8 @@ struct rb_ractor_struct { https://github.com/ruby/ruby/blob/trunk/ractor_core.h#L138 VALUE verbose; VALUE debug; + rb_hook_list_t event_hooks; + struct { struct RVALUE *freelist; struct heap_page *using_page; diff --git a/vm.c b/vm.c index 9ed358f..5fc0338 100644 --- a/vm.c +++ b/vm.c @@ -2588,8 +2588,6 @@ rb_vm_mark(void *ptr) https://github.com/ruby/ruby/blob/trunk/vm.c#L2588 rb_mark_tbl(vm->loading_table); } - rb_hook_list_mark(&vm->global_hooks); - rb_gc_mark_values(RUBY_NSIG, vm->trap_list.cmd); rb_id_table_foreach_values(vm->negative_cme_table, vm_mark_negative_cme, NULL); diff --git a/vm_core.h b/vm_core.h index 304885b..e86026a 100644 --- a/vm_core.h +++ b/vm_core.h @@ -619,9 +619,6 @@ typedef struct rb_vm_struct { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L619 VALUE cmd[RUBY_NSIG]; } trap_list; - /* hook */ - rb_hook_list_t global_hooks; - /* relation table of ensure - rollback for callcc */ struct st_table *ensure_rollback_table; @@ -1973,17 +1970,25 @@ rb_exec_event_hook_orig(rb_execution_context_t *ec, rb_hook_list_t *hooks, rb_ev https://github.com/ruby/ruby/blob/trunk/vm_core.h#L1970 rb_exec_event_hooks(&trace_arg, hooks, pop_p); } +rb_hook_list_t *rb_ractor_hooks(rb_ractor_t *cr);; + static inline rb_hook_list_t * -rb_vm_global_hooks(const rb_execution_context_t *ec) +rb_ec_ractor_hooks(const rb_execution_context_t *ec) { - return &rb_ec_vm_ptr(ec)->global_hooks; + rb_hook_list_t *hooks = rb_ractor_hooks(rb_ec_ractor_ptr(ec)); + if (LIKELY(hooks == NULL)) { + return NULL; + } + else { + return hooks; + } } #define EXEC_EVENT_HOOK(ec_, flag_, self_, id_, called_id_, klass_, data_) \ - EXEC_EVENT_HOOK_ORIG(ec_, rb_vm_global_hooks(ec_), flag_, self_, id_, called_id_, klass_, data_, 0) + EXEC_EVENT_HOOK_ORIG(ec_, rb_ec_ractor_hooks(ec_), flag_, self_, id_, called_id_, klass_, data_, 0) #define EXEC_EVENT_HOOK_AND_POP_FRAME(ec_, flag_, self_, id_, called_id_, klass_, data_) \ - EXEC_EVENT_HOOK_ORIG(ec_, rb_vm_global_hooks(ec_), flag_, self_, id_, called_id_, klass_, data_, 1) + EXEC_EVENT_HOOK_ORIG(ec_, rb_ec_ractor_hooks(ec_), flag_, self_, id_, called_id_, klass_, data_, 1) static inline void rb_exec_event_hook_script_compiled(rb_execution_context_t *ec, const rb_iseq_t *iseq, VALUE eval_script) diff --git a/vm_insnhelper.c b/vm_insnhelper.c index b1673db..90c68d6 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -5308,7 +5308,7 @@ vm_trace(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L5308 return; } else { - rb_hook_list_t *global_hooks = rb_vm_global_hooks(ec); + rb_hook_list_t *global_hooks = rb_ec_ractor_hooks(ec); if (0) { fprintf(stderr, "vm_trace>>%4d (%4x) - %s:%d %s\n", diff --git a/vm_trace.c b/vm_trace.c index 50cdf5f..3835b18 100644 --- a/vm_trace.c +++ b/vm_trace.c @@ -29,6 +29,7 @@ https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L29 #include "mjit.h" #include "ruby/debug.h" #include "vm_core.h" +#include "ruby/ractor.h" #include "builtin.h" @@ -136,7 +137,7 @@ hook_list_connect(VALUE list_owner, rb_hook_list_t *list, rb_event_hook_t *hook, https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L137 static void connect_event_hook(const rb_execution_context_t *ec, rb_event_hook_t *hook) { - rb_hook_list_t *list = rb_vm_global_hooks(ec); + rb_hook_list_t *list = rb_ec_ractor_hooks(ec); hook_list_connect(Qundef, list, hook, TRUE); } @@ -195,7 +196,7 @@ clean_hooks(const rb_execution_context_t *ec, rb_hook_list_t *list) https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L196 } } - if (list == rb_vm_global_hooks(ec)) { + if (list == rb_ec_ractor_hooks(ec)) { /* global events */ update_global_event_hook(list->events); } @@ -220,8 +221,7 @@ clean_hooks_check(const rb_execution_context_t *ec, rb_hook_list_t *list) https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L221 static int remove_event_hook(const rb_execution_context_t *ec, const rb_thread_t *filter_th, rb_event_hook_func_t func, VALUE data) { - rb_vm_t *vm = rb_ec_vm_ptr(ec); - rb_hook_list_t *list = &vm->global_hooks; + rb_hook_list_t *list = rb_ec_ractor_hooks(ec); int ret = 0; rb_event_hook_t *hook = list->hooks; @@ -374,7 +374,7 @@ rb_exec_event_hooks(rb_trace_arg_t *trace_arg, rb_hook_list_t *hooks, int pop_p) https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L374 ec->trace_arg = trace_arg; /* only global hooks */ - exec_hooks_unprotected(ec, rb_vm_global_hooks(ec), trace_arg); + exec_hooks_unprotected(ec, rb_ec_ractor_hooks(ec), trace_arg); ec->trace_arg = prev_trace_arg; } } @@ -708,6 +708,7 @@ typedef struct rb_tp_struct { https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L708 void (*func)(VALUE tpval, void *data); void *data; VALUE proc; + rb_ractor_t *ractor; VALUE self; } rb_tp_t; @@ -1113,7 +1114,9 @@ tp_call_trace(VALUE tpval, rb_trace_arg_t *trace_arg) https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L1114 (*tp->func)(tpval, tp->data); } else { - rb_proc_call_with_block((VALUE)tp->proc, 1, &tpval, Qnil); + if (tp->ractor == NULL || tp->ractor == GET_RACTOR()) { + rb_proc_call_with_block((VALUE)tp->proc, 1, &tpval, Qnil); + } } } @@ -1360,6 +1363,7 @@ tracepoint_new(VALUE klass, rb_thread_t *target_th, rb_event_flag_t events, void https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L1363 TypedData_Get_Struct(tpval, rb_tp_t, &tp_data_type, tp); tp->proc = proc; + tp->ractor = rb_ractor_shareable_p(proc) ? NULL : GET_RACTOR(); tp->func = func; tp->data = data; tp->events = events; @@ -1513,7 +1517,7 @@ tracepoint_stat_s(rb_execution_context_t *ec, VALUE self) https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L1517 rb_vm_t *vm = GET_VM(); VALUE stat = rb_hash_new(); - tracepoint_stat_event_hooks(stat, vm->self, vm->global_hooks.hooks); + tracepoint_stat_event_hooks(stat, vm->self, rb_ec_ractor_hooks(ec)->hooks); /* TODO: thread local hooks */ return stat; -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/