Ractor.make_shareable(a_proc)

Ractor.make_shareable() supports Proc object if (1) a Proc only read outer local variables (no assignments) (2) read outer local variables are shareable. Read local variables are stored in a snapshot, so after making shareable Proc, any assignments are not affeect like that: ```ruby a = 1 pr = Ractor.make_shareable(Proc.new{p a}) pr.call #=> 1 a = 2 pr.call #=> 1 # `a = 2` doesn't affect ``` [Feature #17284]
Merged: https://github.com/ruby/ruby/pull/3722
2020-10-30 00:32:53 +09:00 · 2020-10-30 00:32:53 +09:00 · 5d97bdc2dc · 2020-10-30 03:12:39 +09:00
commit 5d97bdc2dc
parent 502d6d8459
5 changed files with 157 additions and 21 deletions
--- a/bootstraptest/test_ractor.rb
+++ b/bootstraptest/test_ractor.rb
@ -922,6 +922,33 @@ assert_equal 'true', %q{
  [a.frozen?, a[0].frozen?] == [true, false]
 }

+# Ractor.make_shareable(a_proc) makes a proc shareable.
+assert_equal 'true', %q{
+  a = [1, [2, 3], {a: "4"}]
+  pr = Proc.new do
+    a
+  end
+  Ractor.make_shareable(a) # referred value should be shareable
+  Ractor.make_shareable(pr)
+  Ractor.shareable?(pr)
+}
+
+# Ractor.make_shareable(a_proc) makes a proc shareable.
+assert_equal 'can not make a Proc shareable because it accesses outer variables (a).', %q{
+  a = b = nil
+  pr = Proc.new do
+    c = b # assign to a is okay because c is block local variable
+          # reading b is okay
+    a = b # assign to a is not allowed #=> Ractor::Error
+  end
+
+  begin
+    Ractor.make_shareable(pr)
+  rescue => e
+    e.message
+  end
+}
+
 ###
 ### Synchronization tests
 ###
--- a/ractor.c
+++ b/ractor.c
@ -21,6 +21,12 @@ static VALUE rb_eRactorMovedError;
 static VALUE rb_eRactorClosedError;
 static VALUE rb_cRactorMovedObject;

+VALUE
+rb_ractor_error_class(void)
+{
+    return rb_eRactorError;
+}
+
 RUBY_SYMBOL_EXPORT_BEGIN
 // to share with MJIT
 bool ruby_multi_ractor;
@ -2069,28 +2075,44 @@ rb_obj_traverse(VALUE obj,
 }

 static int
-frozen_shareable_p(VALUE obj)
+frozen_shareable_p(VALUE obj, bool *made_shareable)
 {
-    if (!RB_TYPE_P(obj, T_DATA) ||
-        (RTYPEDDATA_P(obj) &&
-         RTYPEDDATA_TYPE(obj)->flags & RUBY_TYPED_FROZEN_SHAREABLE)) {
+    if (!RB_TYPE_P(obj, T_DATA)) {
        return true;
    }
-    else {
-        return false;
+    else if (RTYPEDDATA_P(obj)) {
+        const rb_data_type_t *type = RTYPEDDATA_TYPE(obj);
+        if (type->flags & RUBY_TYPED_FROZEN_SHAREABLE) {
+            return true;
+        }
+        else if (made_shareable && rb_obj_is_proc(obj)) {
+            // special path to make shareable Proc.
+            rb_proc_ractor_make_shareable(obj);
+            *made_shareable = true;
+            VM_ASSERT(RB_OBJ_SHAREABLE_P(obj));
+            return false;
+        }
    }
+
+    return false;
 }

 static enum obj_traverse_iterator_result
 make_shareable_check_shareable(VALUE obj)
 {
    VM_ASSERT(!SPECIAL_CONST_P(obj));
+    bool made_shareable = false;

    if (RB_OBJ_SHAREABLE_P(obj)) {
        return traverse_skip;
    }
-    else if (!frozen_shareable_p(obj)) {
-        rb_raise(rb_eRactorError, "can not make shareable object for %"PRIsVALUE, obj);
+    else if (!frozen_shareable_p(obj, &made_shareable)) {
+        if (made_shareable) {
+            return traverse_skip;
+        }
+        else {
+            rb_raise(rb_eRactorError, "can not make shareable object for %"PRIsVALUE, obj);
+        }
    }

    if (!RB_OBJ_FROZEN_RAW(obj)) {
@ -2099,6 +2121,10 @@ make_shareable_check_shareable(VALUE obj)
        if (UNLIKELY(!RB_OBJ_FROZEN_RAW(obj))) {
            rb_raise(rb_eRactorError, "#freeze does not freeze object correctly");
        }
+
+        if (RB_OBJ_SHAREABLE_P(obj)) {
+            return traverse_skip;
+        }
    }

    return traverse_cont;
@ -2134,7 +2160,7 @@ shareable_p_enter(VALUE obj)
        return traverse_skip;
    }
    else if (RB_OBJ_FROZEN_RAW(obj) &&
-             frozen_shareable_p(obj)) {
+             frozen_shareable_p(obj, NULL)) {
        return traverse_cont;
    }

--- a/vm.c
+++ b/vm.c
@ -960,13 +960,16 @@ rb_proc_dup(VALUE self)

    GetProcPtr(self, src);
    procval = proc_create(rb_cProc, &src->block, src->is_from_method, src->is_lambda);
+    if (RB_OBJ_SHAREABLE_P(self)) FL_SET_RAW(procval, RUBY_FL_SHAREABLE);
    RB_GC_GUARD(self); /* for: body = rb_proc_dup(body) */
    return procval;
 }

 struct collect_outer_variable_name_data {
    VALUE ary;
+    VALUE read_only;
    bool yield;
+    bool isolate;
 };

 static enum rb_id_table_iterator_result
@ -978,40 +981,70 @@ collect_outer_variable_names(ID id, VALUE val, void *ptr)
        data->yield = true;
    }
    else {
-        if (data->ary == Qfalse) data->ary = rb_ary_new();
-        rb_ary_push(data->ary, rb_id2str(id));
+        if (data->isolate ||
+            val == Qtrue /* write */) {
+            if (data->ary == Qfalse) data->ary = rb_ary_new();
+            rb_ary_push(data->ary, rb_id2str(id));
+        }
+        else {
+            if (data->read_only == Qfalse) data->read_only = rb_ary_new();
+            rb_ary_push(data->read_only, rb_id2str(id));
+        }
    }
    return ID_TABLE_CONTINUE;
 }

+VALUE rb_ractor_error_class(void);
+
 static const rb_env_t *
-env_copy(const VALUE *src_ep)
+env_copy(const VALUE *src_ep, VALUE read_only_variables)
 {
    const rb_env_t *src_env = (rb_env_t *)VM_ENV_ENVVAL(src_ep);
+    VM_ASSERT(src_env->ep == src_ep);
+
    VALUE *env_body = ZALLOC_N(VALUE, src_env->env_size); // fill with Qfalse
    VALUE *ep = &env_body[src_env->env_size - 2];

-    VM_ASSERT(src_env->ep == src_ep);
+    if (read_only_variables) {
+        for (int i=0; i<RARRAY_LENINT(read_only_variables); i++) {
+            ID id = SYM2ID(rb_str_intern(RARRAY_AREF(read_only_variables, i)));
+
+            for (unsigned int j=0; j<src_env->iseq->body->local_table_size; j++) {
+                if (id ==  src_env->iseq->body->local_table[j]) {
+                    VALUE v = src_env->env[j];
+                    if (!rb_ractor_shareable_p(v)) {
+                        rb_raise(rb_ractor_error_class(),
+                                 "can not make shareable Proc because it can refere unshareable object %"
+                                 PRIsVALUE" from variable `%s'", rb_inspect(v), rb_id2name(id));
+                    }
+                    env_body[j] = v;
+                    rb_ary_delete_at(read_only_variables, i);
+                    break;
+                }
+            }
+        }
+    }

    ep[VM_ENV_DATA_INDEX_ME_CREF] = src_ep[VM_ENV_DATA_INDEX_ME_CREF];
    ep[VM_ENV_DATA_INDEX_FLAGS]   = src_ep[VM_ENV_DATA_INDEX_FLAGS] | VM_ENV_FLAG_ISOLATED;

    if (!VM_ENV_LOCAL_P(src_ep)) {
        const VALUE *prev_ep = VM_ENV_PREV_EP(src_env->ep);
-        const rb_env_t *new_prev_env = env_copy(prev_ep);
+        const rb_env_t *new_prev_env = env_copy(prev_ep, read_only_variables);
        ep[VM_ENV_DATA_INDEX_SPECVAL] = VM_GUARDED_PREV_EP(new_prev_env->ep);
    }
    else {
        ep[VM_ENV_DATA_INDEX_SPECVAL] = VM_BLOCK_HANDLER_NONE;
    }
+
    return vm_env_new(ep, env_body, src_env->env_size, src_env->iseq);
 }

 static void
-proc_isolate_env(VALUE self, rb_proc_t *proc)
+proc_isolate_env(VALUE self, rb_proc_t *proc, VALUE read_only_variables)
 {
    const struct rb_captured_block *captured = &proc->block.as.captured;
-    const rb_env_t *env = env_copy(captured->ep);
+    const rb_env_t *env = env_copy(captured->ep, read_only_variables);
    *((const VALUE **)&proc->block.as.captured.ep) = env->ep;
    RB_OBJ_WRITTEN(self, Qundef, env);
 }
@ -1019,7 +1052,6 @@ proc_isolate_env(VALUE self, rb_proc_t *proc)
 VALUE
 rb_proc_isolate_bang(VALUE self)
 {
-    // check accesses
    const rb_iseq_t *iseq = vm_proc_iseq(self);

    if (iseq) {
@ -1028,10 +1060,10 @@ rb_proc_isolate_bang(VALUE self)

        if (iseq->body->outer_variables) {
            struct collect_outer_variable_name_data data = {
+                .isolate = true,
                .ary = Qfalse,
                .yield = false,
            };
-
            rb_id_table_foreach(iseq->body->outer_variables, collect_outer_variable_names, (void *)&data);

            if (data.ary != Qfalse) {
@ -1051,10 +1083,11 @@ rb_proc_isolate_bang(VALUE self)
            }
        }

-        proc_isolate_env(self, proc);
+        proc_isolate_env(self, proc, Qfalse);
        proc->is_isolated = TRUE;
    }

+    FL_SET_RAW(self, RUBY_FL_SHAREABLE);
    return self;
 }

@ -1066,6 +1099,53 @@ rb_proc_isolate(VALUE self)
    return dst;
 }

+VALUE
+rb_proc_ractor_make_shareable(VALUE self)
+{
+    const rb_iseq_t *iseq = vm_proc_iseq(self);
+
+    if (iseq) {
+        rb_proc_t *proc = (rb_proc_t *)RTYPEDDATA_DATA(self);
+        if (proc->block.type != block_type_iseq) rb_raise(rb_eRuntimeError, "not supported yet");
+
+        VALUE read_only_variables = Qfalse;
+
+        if (iseq->body->outer_variables) {
+            struct collect_outer_variable_name_data data = {
+                .isolate = false,
+                .ary = Qfalse,
+                .read_only = Qfalse,
+                .yield = false,
+            };
+
+            rb_id_table_foreach(iseq->body->outer_variables, collect_outer_variable_names, (void *)&data);
+
+            if (data.ary != Qfalse) {
+                VALUE str = rb_ary_join(data.ary, rb_str_new2(", "));
+                if (data.yield) {
+                    rb_raise(rb_eArgError, "can not make a Proc shareable because it accesses outer variables (%s) and uses `yield'.",
+                             StringValueCStr(str));
+                }
+                else {
+                    rb_raise(rb_eArgError, "can not make a Proc shareable because it accesses outer variables (%s).",
+                             StringValueCStr(str));
+                }
+            }
+            else if (data.yield) {
+                rb_raise(rb_eArgError, "can not make a Proc shareable because it uses `yield'.");
+            }
+
+            read_only_variables = data.read_only;
+        }
+
+        proc_isolate_env(self, proc, read_only_variables);
+        proc->is_isolated = TRUE;
+    }
+
+    FL_SET_RAW(self, RUBY_FL_SHAREABLE);
+    return self;
+}
+
 MJIT_FUNC_EXPORTED VALUE
 rb_vm_make_proc_lambda(const rb_execution_context_t *ec, const struct rb_captured_block *captured, VALUE klass, int8_t is_lambda)
 {
--- a/vm_core.h
+++ b/vm_core.h
@ -1065,6 +1065,7 @@ typedef struct {

 VALUE rb_proc_isolate(VALUE self);
 VALUE rb_proc_isolate_bang(VALUE self);
+VALUE rb_proc_ractor_make_shareable(VALUE self);

 typedef struct {
    VALUE flags; /* imemo header */
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@ -2688,13 +2688,15 @@ vm_call_bmethod_body(rb_execution_context_t *ec, struct rb_calling_info *calling
    VALUE val;
    const struct rb_callcache *cc = cd->cc;
    const rb_callable_method_entry_t *cme = vm_cc_cme(cc);
+    VALUE procv = cme->def->body.bmethod.proc;

-    if (cme->def->body.bmethod.defined_ractor != rb_ec_ractor_ptr(ec)->self) {
+    if (!RB_OBJ_SHAREABLE_P(procv) &&
+        cme->def->body.bmethod.defined_ractor != rb_ec_ractor_ptr(ec)->self) {
        rb_raise(rb_eRuntimeError, "defined in a different Ractor");
    }

    /* control block frame */
-    GetProcPtr(cme->def->body.bmethod.proc, proc);
+    GetProcPtr(procv, proc);
    val = rb_vm_invoke_bmethod(ec, proc, calling->recv, calling->argc, argv, calling->kw_splat, calling->block_handler, vm_cc_cme(cc));

    return val;