Put the reference chain in RactorError#detailed_message

Author: etiennebarrie

bootstraptest/test_ractor.rb

@@ -1006,7 +1006,7 @@ def inspect = "#<Foo @ivar=#{@ivar.inspect}>"
   begin
     Ractor.make_shareable(Bar.new(Foo.new))
   rescue Ractor::Error
-    $!.to_s.lines[1..].join
+    $!.detailed_message.lines[1..].join
   end
 }
 

ractor.c

@@ -33,6 +33,8 @@ static VALUE rb_eRactorMovedError;
 static VALUE rb_eRactorClosedError;
 static VALUE rb_cRactorMovedObject;
 
+static ID id_reference_chain;
+
 static void vm_ractor_blocking_cnt_inc(rb_vm_t *vm, rb_ractor_t *r, const char *file, int line);
 
 
@@ -1038,13 +1040,37 @@ ractor_moved_missing(int argc, VALUE *argv, VALUE self)
  *
  */
 
+/*
+ * call-seq:
+ *   detailed_message(highlight: false, **kwargs) -> string
+ *
+ * Returns the message string with the reference chain appended.
+ */
+static VALUE
+ractor_error_detailed_message(int argc, VALUE *argv, VALUE exc)
+{
+    // Call super to get the base detailed_message
+    VALUE base_message = rb_call_super_kw(argc, argv, RB_PASS_CALLED_KEYWORDS);
+
+    VALUE chain = rb_attr_get(exc, id_reference_chain);
+    if (NIL_P(chain)) {
+        return base_message;
+    }
+
+    return rb_sprintf("%"PRIsVALUE"%"PRIsVALUE, base_message, chain);
+}
+
 void
 Init_Ractor(void)
 {
     rb_cRactor = rb_define_class("Ractor", rb_cObject);
     rb_undef_alloc_func(rb_cRactor);
 
+    id_reference_chain = rb_intern_const("reference_chain");
+
     rb_eRactorError          = rb_define_class_under(rb_cRactor, "Error", rb_eRuntimeError);
+    rb_define_method(rb_eRactorError, "detailed_message", ractor_error_detailed_message, -1);
+
     rb_eRactorIsolationError = rb_define_class_under(rb_cRactor, "IsolationError", rb_eRactorError);
     rb_eRactorRemoteError    = rb_define_class_under(rb_cRactor, "RemoteError", rb_eRactorError);
     rb_eRactorMovedError     = rb_define_class_under(rb_cRactor, "MovedError",  rb_eRactorError);
@@ -1631,13 +1657,16 @@ rb_ractor_make_shareable(VALUE obj)
     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);
+            if (!NIL_P(chain)) {
+                rb_ivar_set(exception, id_reference_chain, chain);
+            }
             rb_exc_raise(exception);
         }
-        rb_raise(rb_eRactorError, "can not make shareable object for %+"PRIsVALUE"%"PRIsVALUE, obj, chain);
+        exception = rb_exc_new3(rb_eRactorError, rb_sprintf("can not make shareable object for %+"PRIsVALUE, obj));
+        if (!NIL_P(chain)) {
+            rb_ivar_set(exception, id_reference_chain, chain);
+        }
+        rb_exc_raise(exception);
     }
     RB_GC_GUARD(chain);
     RB_GC_GUARD(exception);
@@ -1671,6 +1700,23 @@ rb_ractor_ensure_main_ractor(const char *msg)
     }
 }
 
+NORETURN(void rb_ractor_raise_isolation_error_with_chain(VALUE klass, VALUE chain, const char *fmt, ...));
+
+void
+rb_ractor_raise_isolation_error_with_chain(VALUE klass, VALUE chain, const char *fmt, ...)
+{
+    va_list args;
+    va_start(args, fmt);
+    VALUE message = rb_vsprintf(fmt, args);
+    va_end(args);
+
+    VALUE exception = rb_exc_new_str(klass, message);
+    if (!NIL_P(chain)) {
+        rb_ivar_set(exception, id_reference_chain, chain);
+    }
+    rb_exc_raise(exception);
+}
+
 static enum obj_traverse_iterator_result
 shareable_p_enter(VALUE obj, struct obj_traverse_data *data)
 {
@@ -2782,6 +2828,34 @@ rb_ractor_isolation_violation(const char *fmt, ...)
     rb_ractor_isolation_violation_str(message);
 }
 
+void
+rb_ractor_isolation_violation_with_chain(VALUE chain, const char *fmt, ...)
+{
+    va_list args;
+    va_start(args, fmt);
+    VALUE message = rb_vsprintf(fmt, args);
+    va_end(args);
+
+    if (rb_thread_ractor_isolation_check_p()) {
+        // Warning path: append the chain inline so it shows next to the
+        // warning. There's no exception object to attach @reference_chain to.
+        if (!NIL_P(chain)) {
+            rb_str_append(message, chain);
+        }
+        rb_category_warn(RB_WARN_CATEGORY_RACTOR_ISOLATION, "%s", StringValueCStr(message));
+        return;
+    }
+
+    // Raise path: keep the chain on the exception's @reference_chain so it
+    // surfaces via Exception#detailed_message (matches the design of
+    // rb_ractor_raise_isolation_error_with_chain).
+    VALUE exception = rb_exc_new_str(rb_eRactorIsolationError, message);
+    if (!NIL_P(chain)) {
+        rb_ivar_set(exception, id_reference_chain, chain);
+    }
+    rb_exc_raise(exception);
+}
+
 static VALUE
 ractor_check_isolation_ensure(VALUE prev_state)
 {

ractor_core.h

@@ -183,10 +183,18 @@ bool rb_thread_ractor_isolation_check_p(void);
 PRINTF_ARGS(void rb_ractor_isolation_violation(const char *fmt, ...), 1, 2);
 void rb_ractor_isolation_violation_str(VALUE message);
 
+/* Like rb_ractor_isolation_violation, but also carries the reference chain
+ * built by rb_ractor_shareable_p_continue. In check_isolation mode the chain
+ * is appended to the warning message; otherwise it is attached to the raised
+ * Ractor::IsolationError as @reference_chain (so Exception#detailed_message
+ * surfaces it). */
+PRINTF_ARGS(void rb_ractor_isolation_violation_with_chain(VALUE chain, const char *fmt, ...), 2, 3);
+
 RUBY_SYMBOL_EXPORT_BEGIN
 void rb_ractor_finish_marking(void);
 
 bool rb_ractor_shareable_p_continue(VALUE obj, VALUE *chain);
+NORETURN(void rb_ractor_raise_isolation_error_with_chain(VALUE klass, VALUE chain, const char *fmt, ...));
 
 // THIS FUNCTION SHOULD NOT CALL WHILE INCREMENTAL MARKING!!
 // This function is for T_DATA::free_func

test/ruby/test_ractor.rb

@@ -298,40 +298,40 @@ def initialize
         @unshareable = -> {}
       end
     end.new
-    assert_unshareable(obj, /from instance variable @unshareable of an instance of #<Class:/)
+    assert_unshareable(obj, detailed_message: /from instance variable @unshareable of an instance of #<Class:/)
   end
 
   def test_error_includes_array_index
-    assert_unshareable([0, -> {}], /from Array element at index 1/)
+    assert_unshareable([0, -> {}], detailed_message: /from Array element at index 1/)
   end
 
   def test_error_includes_hash_key_and_value
-    assert_unshareable({ unshareable: -> {} }, /from Hash value at key :unshareable/)
+    assert_unshareable({ unshareable: -> {} }, detailed_message: /from Hash value at key :unshareable/)
   end
 
   def test_error_includes_hash_unshareable_key
-    assert_unshareable({ -> {} => true }, /from Hash key #<Proc:0x[[:xdigit:]]+ #{__FILE__}:#{__LINE__}/)
+    assert_unshareable({ -> {} => true }, detailed_message: /from Hash key #<Proc:0x[[:xdigit:]]+ #{__FILE__}:#{__LINE__}/)
   end
 
   def test_error_includes_hash_default_proc
     h = Hash.new {}
-    assert_unshareable(h, /from Hash default proc/)
+    assert_unshareable(h, detailed_message: /from Hash default proc/)
   end
 
   def test_error_includes_hash_default_value
     h = Hash.new(Mutex.new)
-    assert_unshareable(h, /from Hash default value/, exception: Ractor::Error)
+    assert_unshareable(h, detailed_message: /from Hash default value/, exception: Ractor::Error)
   end
 
   S = Struct.new(:member)
   def test_error_includes_struct_member
     s = S.new(-> {})
-    assert_unshareable(s, /from member :member of an instance of TestRactor::S/)
+    assert_unshareable(s, detailed_message: /from member :member of an instance of TestRactor::S/)
   end
 
   def test_error_includes_block_self
     pr = -> {}
-    assert_unshareable(pr, /from block's self \(an instance of #{self.class.name}\)/)
+    assert_unshareable(pr, detailed_message: /from block's self \(an instance of #{self.class.name}\)/)
   end
 
   def test_error_wraps_freeze_error
@@ -357,7 +357,7 @@ def test_error_for_module_instance_variable
         $!
       end.value
       assert_kind_of Ractor::IsolationError, e
-      assert_match(/from Hash default proc/, e.message)
+      assert_match(/from Hash default proc/, e.detailed_message)
     RUBY
   end
 
@@ -373,7 +373,7 @@ module ModuleWithUnshareableConstant
         $!
       end.value
       assert_kind_of(Ractor::IsolationError, e)
-      assert_match(/from Hash default proc/, e.message)
+      assert_match(/from Hash default proc/, e.detailed_message)
     RUBY
   end
 
@@ -383,11 +383,12 @@ def assert_make_shareable(obj)
     assert Ractor.shareable?(obj), "object didn't become shareable"
   end
 
-  def assert_unshareable(obj, msg=nil, exception: Ractor::IsolationError)
+  def assert_unshareable(obj, msg=//, detailed_message: nil, exception: Ractor::IsolationError)
     refute Ractor.shareable?(obj), "object is already shareable"
     e = assert_raise_with_message(exception, msg) do
       Ractor.make_shareable(obj)
     end
+    assert_match(detailed_message, e.detailed_message) if detailed_message
     refute Ractor.shareable?(obj), "despite raising, object became shareable"
     e
   end

variable.c

@@ -1470,9 +1470,9 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef)
                     UNLIKELY(rb_ractor_isolation_check_active())) {
                 VALUE chain = Qnil;
                 if (!rb_ractor_shareable_p_continue(val, &chain)) {
-                    rb_ractor_isolation_violation(
-                            "can not get unshareable values from instance variables of classes/modules from non-main Ractors (%"PRIsVALUE" from %"PRIsVALUE")%"PRIsVALUE,
-                            rb_id2str(id), obj, chain);
+                    rb_ractor_isolation_violation_with_chain(chain,
+                            "can not get unshareable values from instance variables of classes/modules from non-main Ractors (%"PRIsVALUE" from %"PRIsVALUE")",
+                            rb_id2str(id), obj);
                 }
             }
             return val;
@@ -3448,9 +3448,9 @@ rb_const_get_0(VALUE klass, ID id, int exclude, int recurse, int visibility)
         if (UNLIKELY(rb_ractor_isolation_check_active())) {
             VALUE chain = Qnil;
             if (!rb_ractor_shareable_p_continue(c, &chain)) {
-                rb_ractor_isolation_violation(
-                        "can not access non-shareable objects in constant %"PRIsVALUE"::%"PRIsVALUE" by non-main Ractor.%"PRIsVALUE,
-                        rb_class_path(found_in), rb_id2str(id), chain);
+                rb_ractor_isolation_violation_with_chain(chain,
+                        "can not access non-shareable objects in constant %"PRIsVALUE"::%"PRIsVALUE" by non-main Ractor.",
+                        rb_class_path(found_in), rb_id2str(id));
             }
         }
         return c;

vm.c

@@ -1584,9 +1584,9 @@ rb_proc_ractor_make_shareable(VALUE self, VALUE replace_self)
 {
     VALUE chain = Qnil;
     if (!rb_proc_ractor_make_shareable_continue(self, replace_self, &chain)) {
-        rb_ractor_isolation_violation(
-                    "Proc's self is not shareable: %" PRIsVALUE "%"PRIsVALUE,
-                    self, chain);
+        rb_ractor_isolation_violation_with_chain(chain,
+                    "Proc's self is not shareable: %" PRIsVALUE,
+                    self);
     }
     return self;
 }

vm_insnhelper.c

@@ -1145,9 +1145,9 @@ vm_get_ev_const(rb_execution_context_t *ec, VALUE orig_klass, ID id, bool allow_
                             if (UNLIKELY(rb_ractor_isolation_check_active())) {
                                 VALUE chain = Qnil;
                                 if (!rb_ractor_shareable_p_continue(val, &chain)) {
-                                    rb_ractor_isolation_violation(
-                                             "can not access non-shareable objects in constant %"PRIsVALUE"::%"PRIsVALUE" by non-main Ractor.%"PRIsVALUE,
-                                             rb_class_path(klass), rb_id2str(id), chain);
+                                    rb_ractor_isolation_violation_with_chain(chain,
+                                             "can not access non-shareable objects in constant %"PRIsVALUE"::%"PRIsVALUE" by non-main Ractor.",
+                                             rb_class_path(klass), rb_id2str(id));
                                 }
                             }
                             return val;