Show chain of references in Ractor errors

Improve the messages of exceptions raised by the Ractor implementation.

When an object fails to be made shareable with `Ractor.make_shareable`
or when an unshareable object is accessed through module constants or
module instance variables, the error message now includes the chain of
references that leads to the unshareable value.

Author: etiennebarrie

bootstraptest/test_ractor.rb

@@ -986,6 +986,50 @@ def self.fstr = @fstr
   a + b + c + d + e + f
 }
 
+assert_equal "  from hash default value\n" \
+    "  from instance variable @ivar\n" \
+    "  from block self #<Foo @ivar={}>\n" \
+    "  from hash default value\n" \
+    "  from instance variable @ivar\n" \
+    "  from instance variable @foo", %q{
+  class Foo
+    def initialize
+      @ivar = Hash.new { |h, k| h[k] = [] } # the default proc holds self, an instance of Foo
+    end
+    def inspect = "#<Foo @ivar=#{@ivar.inspect}>"
+  end
+
+  class Bar
+    def initialize
+      @foo = Foo.new # holds an instance of an object that owns a Proc
+    end
+    def inspect = "#<Bar @foo=#{@foo.inspect}>"
+  end
+
+  class Proc
+    def inspect = "#<Proc>"
+  end
+
+  begin
+    Ractor.make_shareable Bar.new
+  rescue Ractor::Error
+    $!.to_s.lines[1..].join.chomp
+  end
+}
+
+assert_equal '[true, true]', %q{
+  class Foo
+    undef_method :freeze
+  end
+
+  begin
+    Ractor.make_shareable Foo.new
+  rescue Ractor::Error
+    cause = $!.cause
+    [cause.class == NoMethodError, cause.name == :freeze]
+  end
+}
+
 assert_equal '["instance-variable", "instance-variable", nil]', %q{
   class C
     @iv1 = ""

include/ruby/ractor.h

@@ -248,7 +248,7 @@ RBIMPL_SYMBOL_EXPORT_END()
 static inline bool
 rb_ractor_shareable_p(VALUE obj)
 {
-    bool rb_ractor_shareable_p_continue(VALUE obj);
+    bool rb_ractor_shareable_p_continue(VALUE obj, VALUE *chain);
 
     if (RB_SPECIAL_CONST_P(obj)) {
         return true;
@@ -257,7 +257,7 @@ rb_ractor_shareable_p(VALUE obj)
         return true;
     }
     else {
-        return rb_ractor_shareable_p_continue(obj);
+        return rb_ractor_shareable_p_continue(obj, NULL);
     }
 }
 

ractor.c

@@ -1211,7 +1211,8 @@ enum obj_traverse_iterator_result {
     traverse_stop,
 };
 
-typedef enum obj_traverse_iterator_result (*rb_obj_traverse_enter_func)(VALUE obj);
+struct obj_traverse_data;
+typedef enum obj_traverse_iterator_result (*rb_obj_traverse_enter_func)(VALUE obj, struct obj_traverse_data *data);
 typedef enum obj_traverse_iterator_result (*rb_obj_traverse_leave_func)(VALUE obj);
 typedef enum obj_traverse_iterator_result (*rb_obj_traverse_final_func)(VALUE obj);
 
@@ -1222,15 +1223,35 @@ struct obj_traverse_data {
     rb_obj_traverse_leave_func leave_func;
 
     st_table *rec;
-    VALUE rec_hash;
+    VALUE rec_hash;  // objects seen during traversal
+    VALUE *chain; // reference chain string built during unwinding (NULL if not needed)
+    VALUE *exception; // exception raised trying to freeze an object
 };
 
-
 struct obj_traverse_callback_data {
     bool stop;
     struct obj_traverse_data *data;
 };
 
+static inline void
+chain_append(VALUE *chain_ptr, const char *fmt, ...)
+{
+    if (!chain_ptr) return;
+
+    va_list args;
+    va_start(args, fmt);
+
+    if (NIL_P(*chain_ptr)) {
+        *chain_ptr = rb_str_new_cstr("\n");
+    }
+    else {
+        rb_str_cat_cstr(*chain_ptr, "\n");
+    }
+    rb_str_vcatf(*chain_ptr, fmt, args);
+
+    va_end(args);
+}
+
 static int obj_traverse_i(VALUE obj, struct obj_traverse_data *data);
 
 static int
@@ -1239,11 +1260,13 @@ obj_hash_traverse_i(VALUE key, VALUE val, VALUE ptr)
     struct obj_traverse_callback_data *d = (struct obj_traverse_callback_data *)ptr;
 
     if (obj_traverse_i(key, d->data)) {
+        chain_append(d->data->chain, "  from hash key %+"PRIsVALUE, key);
         d->stop = true;
         return ST_STOP;
     }
 
     if (obj_traverse_i(val, d->data)) {
+        chain_append(d->data->chain, "  from hash value at key %+"PRIsVALUE, key);
         d->stop = true;
         return ST_STOP;
     }
@@ -1277,6 +1300,7 @@ obj_traverse_ivar_foreach_i(ID key, VALUE val, st_data_t ptr)
     struct obj_traverse_callback_data *d = (struct obj_traverse_callback_data *)ptr;
 
     if (obj_traverse_i(val, d->data)) {
+        chain_append(d->data->chain, "  from instance variable %"PRIsVALUE, rb_id2str(key));
         d->stop = true;
         return ST_STOP;
     }
@@ -1289,7 +1313,7 @@ obj_traverse_i(VALUE obj, struct obj_traverse_data *data)
 {
     if (RB_SPECIAL_CONST_P(obj)) return 0;
 
-    switch (data->enter_func(obj)) {
+    switch (data->enter_func(obj, data)) {
       case traverse_cont: break;
       case traverse_skip: return 0; // skip children
       case traverse_stop: return 1; // stop search
@@ -1306,7 +1330,9 @@ obj_traverse_i(VALUE obj, struct obj_traverse_data *data)
         .data = data,
     };
     rb_ivar_foreach(obj, obj_traverse_ivar_foreach_i, (st_data_t)&d);
-    if (d.stop) return 1;
+    if (d.stop) {
+        return 1;
+    }
 
     switch (BUILTIN_TYPE(obj)) {
       // no child node
@@ -1328,14 +1354,21 @@ obj_traverse_i(VALUE obj, struct obj_traverse_data *data)
 
             for (int i = 0; i < RARRAY_LENINT(obj); i++) {
                 VALUE e = rb_ary_entry(obj, i);
-                if (obj_traverse_i(e, data)) return 1;
+                if (obj_traverse_i(e, data)) {
+                    chain_append(data->chain, "  from array element at index %d", i);
+                    return 1;
+                }
             }
         }
         break;
 
       case T_HASH:
         {
-            if (obj_traverse_i(RHASH_IFNONE(obj), data)) return 1;
+            const VALUE ifnone = RHASH_IFNONE(obj);
+            if (obj_traverse_i(ifnone, data)) {
+                chain_append(data->chain, "  from hash default value");
+                return 1;
+            }
 
             struct obj_traverse_callback_data d = {
                 .stop = false,
@@ -1352,7 +1385,12 @@ obj_traverse_i(VALUE obj, struct obj_traverse_data *data)
             const VALUE *ptr = RSTRUCT_CONST_PTR(obj);
 
             for (long i=0; i<len; i++) {
-                if (obj_traverse_i(ptr[i], data)) return 1;
+                if (obj_traverse_i(ptr[i], data)) {
+                    VALUE members = rb_struct_members(obj);
+                    VALUE member_name = rb_array_const_ptr(members)[i];
+                    chain_append(data->chain, "  from struct member %+"PRIsVALUE, member_name);
+                    return 1;
+                }
             }
         }
         break;
@@ -1423,15 +1461,21 @@ static int
 rb_obj_traverse(VALUE obj,
                 rb_obj_traverse_enter_func enter_func,
                 rb_obj_traverse_leave_func leave_func,
-                rb_obj_traverse_final_func final_func)
+                rb_obj_traverse_final_func final_func,
+                VALUE *chain,
+                VALUE *exception)
 {
     struct obj_traverse_data data = {
         .enter_func = enter_func,
         .leave_func = leave_func,
         .rec = NULL,
+        .chain = chain,
+        .exception = exception,
     };
 
-    if (obj_traverse_i(obj, &data)) return 1;
+    if (obj_traverse_i(obj, &data)) {
+        return 1;
+    }
     if (final_func && data.rec) {
         struct rb_obj_traverse_final_data f = {final_func, 0};
         st_foreach(data.rec, obj_traverse_final_i, (st_data_t)&f);
@@ -1456,14 +1500,45 @@ allow_frozen_shareable_p(VALUE obj)
     return false;
 }
 
+static VALUE
+try_freeze(VALUE obj)
+{
+    rb_funcall(obj, idFreeze, 0);
+    return Qtrue;
+}
+
+struct rescue_freeze_data {
+    VALUE exception;
+};
+
+static VALUE
+rescue_freeze(VALUE data, VALUE freeze_exception)
+{
+    struct rescue_freeze_data *rescue_freeze_data = (struct rescue_freeze_data *)data;
+    VALUE exception = rb_exc_new3(rb_eRactorError, rb_str_new_cstr("raised calling #freeze"));
+    rb_ivar_set(exception, rb_intern("cause"), freeze_exception);
+    rescue_freeze_data->exception = exception;
+    return Qfalse;
+}
+
 static enum obj_traverse_iterator_result
-make_shareable_check_shareable_freeze(VALUE obj, enum obj_traverse_iterator_result result)
+make_shareable_check_shareable_freeze(VALUE obj, enum obj_traverse_iterator_result result, struct obj_traverse_data *data)
 {
     if (!RB_OBJ_FROZEN_RAW(obj)) {
-        rb_funcall(obj, idFreeze, 0);
+        struct rescue_freeze_data rescue_freeze_data = { 0 };
+        if (!rb_rescue(try_freeze, obj, rescue_freeze, (VALUE)&rescue_freeze_data)) {
+            if (data->exception) {
+                *data->exception = rescue_freeze_data.exception;
+            }
+            return traverse_stop;
+        }
 
         if (UNLIKELY(!RB_OBJ_FROZEN_RAW(obj))) {
-            rb_raise(rb_eRactorError, "#freeze does not freeze object correctly");
+            VALUE exception = rb_exc_new3(rb_eRactorError, rb_str_new_cstr("#freeze does not freeze object correctly"));
+            if (data->exception) {
+                *data->exception = exception;
+            }
+            return traverse_stop;
         }
 
         if (RB_OBJ_SHAREABLE_P(obj)) {
@@ -1477,7 +1552,7 @@ make_shareable_check_shareable_freeze(VALUE obj, enum obj_traverse_iterator_resu
 static int obj_refer_only_shareables_p(VALUE obj);
 
 static enum obj_traverse_iterator_result
-make_shareable_check_shareable(VALUE obj)
+make_shareable_check_shareable(VALUE obj, struct obj_traverse_data *data)
 {
     VM_ASSERT(!SPECIAL_CONST_P(obj));
 
@@ -1490,7 +1565,8 @@ make_shareable_check_shareable(VALUE obj)
 
         if (type->flags & RUBY_TYPED_FROZEN_SHAREABLE_NO_REC) {
             if (obj_refer_only_shareables_p(obj)) {
-                make_shareable_check_shareable_freeze(obj, traverse_skip);
+                enum obj_traverse_iterator_result result = make_shareable_check_shareable_freeze(obj, traverse_skip, data);
+                if (result == traverse_stop) return traverse_stop;
                 RB_OBJ_SET_SHAREABLE(obj);
                 return traverse_skip;
             }
@@ -1500,11 +1576,19 @@ make_shareable_check_shareable(VALUE obj)
             }
         }
         else if (rb_obj_is_proc(obj)) {
-            rb_proc_ractor_make_shareable(obj, Qundef);
+            if (!rb_proc_ractor_make_shareable_continue(obj, Qundef, data->chain)) {
+                rb_proc_t *proc = (rb_proc_t *)RTYPEDDATA_DATA(obj);
+                if (proc->block.type != block_type_iseq) rb_raise(rb_eRuntimeError, "not supported yet");
+
+                if (data->exception) {
+                    *data->exception = rb_exc_new3(rb_eRactorIsolationError, rb_sprintf("Proc's self is not shareable: %" PRIsVALUE, obj));
+                }
+                return traverse_stop;
+            }
             return traverse_cont;
         }
         else {
-            rb_raise(rb_eRactorError, "can not make shareable object for %+"PRIsVALUE, obj);
+            return traverse_stop;
         }
     }
 
@@ -1529,7 +1613,7 @@ make_shareable_check_shareable(VALUE obj)
         break;
     }
 
-    return make_shareable_check_shareable_freeze(obj, traverse_cont);
+    return make_shareable_check_shareable_freeze(obj, traverse_cont, data);
 }
 
 static enum obj_traverse_iterator_result
@@ -1546,9 +1630,20 @@ mark_shareable(VALUE obj)
 VALUE
 rb_ractor_make_shareable(VALUE obj)
 {
-    rb_obj_traverse(obj,
-                    make_shareable_check_shareable,
-                    null_leave, mark_shareable);
+    VALUE chain = Qnil;
+    VALUE exception = Qfalse;
+    if (rb_obj_traverse(obj, make_shareable_check_shareable, null_leave, mark_shareable, &chain, &exception)) {
+        if (exception) {
+            VALUE id_mesg = rb_intern("mesg");
+            VALUE message = rb_attr_get(exception, id_mesg);
+            message = rb_sprintf("%"PRIsVALUE"%"PRIsVALUE, message, chain);
+            rb_ivar_set(exception, id_mesg, message);
+            rb_exc_raise(exception);
+        }
+        rb_raise(rb_eRactorError, "can not make shareable object for %+"PRIsVALUE"%"PRIsVALUE, obj, chain);
+    }
+    RB_GC_GUARD(chain);
+    RB_GC_GUARD(exception);
     return obj;
 }
 
@@ -1579,7 +1674,7 @@ rb_ractor_ensure_main_ractor(const char *msg)
 }
 
 static enum obj_traverse_iterator_result
-shareable_p_enter(VALUE obj)
+shareable_p_enter(VALUE obj, struct obj_traverse_data *data)
 {
     if (RB_OBJ_SHAREABLE_P(obj)) {
         return traverse_skip;
@@ -1600,11 +1695,9 @@ shareable_p_enter(VALUE obj)
 }
 
 bool
-rb_ractor_shareable_p_continue(VALUE obj)
+rb_ractor_shareable_p_continue(VALUE obj, VALUE *chain)
 {
-    if (rb_obj_traverse(obj,
-                        shareable_p_enter, null_leave,
-                        mark_shareable)) {
+    if (rb_obj_traverse(obj, shareable_p_enter, null_leave, mark_shareable, chain, NULL)) {
         return false;
     }
     else {
@@ -1620,7 +1713,7 @@ rb_ractor_setup_belonging(VALUE obj)
 }
 
 static enum obj_traverse_iterator_result
-reset_belonging_enter(VALUE obj)
+reset_belonging_enter(VALUE obj, struct obj_traverse_data *data)
 {
     if (rb_ractor_shareable_p(obj)) {
         return traverse_skip;
@@ -1642,7 +1735,7 @@ static VALUE
 ractor_reset_belonging(VALUE obj)
 {
 #if RACTOR_CHECK_MODE > 0
-    rb_obj_traverse(obj, reset_belonging_enter, null_leave, NULL);
+    rb_obj_traverse(obj, reset_belonging_enter, null_leave, NULL, NULL, NULL);
 #endif
     return obj;
 }

ractor_core.h

@@ -149,7 +149,7 @@ st_table *rb_ractor_targeted_hooks(rb_ractor_t *cr);
 RUBY_SYMBOL_EXPORT_BEGIN
 void rb_ractor_finish_marking(void);
 
-bool rb_ractor_shareable_p_continue(VALUE obj);
+bool rb_ractor_shareable_p_continue(VALUE obj, VALUE *chain);
 
 // THIS FUNCTION SHOULD NOT CALL WHILE INCREMENTAL MARKING!!
 // This function is for T_DATA::free_func