Skip to content

Commit

Permalink
TracePoint.new(&block) should be ractor-local
Browse files Browse the repository at this point in the history
TracePoint should be ractor-local because the Proc can violate the
Ractor-safe.
  • Loading branch information
ko1 committed Dec 21, 2020
1 parent c7a5cc2 commit a295036
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 17 deletions.
13 changes: 13 additions & 0 deletions bootstraptest/test_ractor.rb
Expand Up @@ -1199,6 +1199,19 @@ class C
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|
Expand Down
1 change: 1 addition & 0 deletions common.mk
Expand Up @@ -16316,6 +16316,7 @@ vm_trace.$(OBJEXT): {$(VPATH)}mjit.h
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
Expand Down
8 changes: 8 additions & 0 deletions ractor.c
Expand Up @@ -197,6 +197,7 @@ ractor_mark(void *ptr)
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;
Expand Down Expand Up @@ -230,6 +231,7 @@ ractor_free(void *ptr)
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);
}

Expand Down Expand Up @@ -2126,6 +2128,12 @@ rb_ractor_stderr_set(VALUE err)
}
}

rb_hook_list_t *
rb_ractor_hooks(rb_ractor_t *cr)
{
return &cr->event_hooks;
}

/// traverse function

// 2: stop search
Expand Down
2 changes: 2 additions & 0 deletions ractor_core.h
Expand Up @@ -138,6 +138,8 @@ struct rb_ractor_struct {
VALUE verbose;
VALUE debug;

rb_hook_list_t event_hooks;

struct {
struct RVALUE *freelist;
struct heap_page *using_page;
Expand Down
2 changes: 0 additions & 2 deletions vm.c
Expand Up @@ -2588,8 +2588,6 @@ rb_vm_mark(void *ptr)
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);
Expand Down
19 changes: 12 additions & 7 deletions vm_core.h
Expand Up @@ -619,9 +619,6 @@ typedef struct rb_vm_struct {
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;

Expand Down Expand Up @@ -1973,17 +1970,25 @@ rb_exec_event_hook_orig(rb_execution_context_t *ec, rb_hook_list_t *hooks, rb_ev
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)
Expand Down
2 changes: 1 addition & 1 deletion vm_insnhelper.c
Expand Up @@ -5308,7 +5308,7 @@ vm_trace(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp)
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",
Expand Down
18 changes: 11 additions & 7 deletions vm_trace.c
Expand Up @@ -29,6 +29,7 @@
#include "mjit.h"
#include "ruby/debug.h"
#include "vm_core.h"
#include "ruby/ractor.h"

#include "builtin.h"

Expand Down Expand Up @@ -136,7 +137,7 @@ hook_list_connect(VALUE list_owner, rb_hook_list_t *list, rb_event_hook_t *hook,
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);
}

Expand Down Expand Up @@ -195,7 +196,7 @@ clean_hooks(const rb_execution_context_t *ec, rb_hook_list_t *list)
}
}

if (list == rb_vm_global_hooks(ec)) {
if (list == rb_ec_ractor_hooks(ec)) {
/* global events */
update_global_event_hook(list->events);
}
Expand All @@ -220,8 +221,7 @@ clean_hooks_check(const rb_execution_context_t *ec, rb_hook_list_t *list)
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;

Expand Down Expand Up @@ -374,7 +374,7 @@ rb_exec_event_hooks(rb_trace_arg_t *trace_arg, rb_hook_list_t *hooks, int pop_p)

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;
}
}
Expand Down Expand Up @@ -708,6 +708,7 @@ typedef struct rb_tp_struct {
void (*func)(VALUE tpval, void *data);
void *data;
VALUE proc;
rb_ractor_t *ractor;
VALUE self;
} rb_tp_t;

Expand Down Expand Up @@ -1113,7 +1114,9 @@ tp_call_trace(VALUE tpval, rb_trace_arg_t *trace_arg)
(*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);
}
}
}

Expand Down Expand Up @@ -1360,6 +1363,7 @@ tracepoint_new(VALUE klass, rb_thread_t *target_th, rb_event_flag_t events, void
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;
Expand Down Expand Up @@ -1513,7 +1517,7 @@ tracepoint_stat_s(rb_execution_context_t *ec, VALUE self)
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;
Expand Down

0 comments on commit a295036

Please sign in to comment.