Skip to content

Commit 9a2ffd8

Browse files
committed
Fix sending backtraces across ractors
[Bug #21818] Currently exceptions can be sent across ractors, but because of a limitation in the TypedData API, the exception backtrace is duped as an empty backtrace. The problem is that backtraces are embedded objects, hence the classic `rb_class_alloc(klass)` API is insufficient because we need to know the size of the Backtrace object we're duping to instantiate the copy. This is worked around by changing Ractors to call `#clone` on objects rather than use `rb_obj_clone`, and to implement `Thread::Backtrace#clone` to properly clone the variable size object.
1 parent 3f1aced commit 9a2ffd8

7 files changed

Lines changed: 140 additions & 17 deletions

File tree

bootstraptest/test_ractor.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ def test n
507507
}
508508

509509
# To copy the object, now Marshal#dump is used
510-
assert_match /can not copy unshareable object/, %q{
510+
assert_match /can't clone unshareable instance of Thread/, %q{
511511
obj = Thread.new{}
512512
begin
513513
r = Ractor.new obj do |msg|

defs/id.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ firstline, predefined = __LINE__+1, %[\
2828
send
2929
__send__
3030
__recursive_key__
31+
clone
3132
initialize
3233
initialize_copy
3334
initialize_clone

depend

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19980,6 +19980,7 @@ vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/compilers.h
1998019980
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/error.h
1998119981
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/gc.h
1998219982
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/imemo.h
19983+
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/object.h
1998319984
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
1998419985
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/serial.h
1998519986
vm_backtrace.$(OBJEXT): $(top_srcdir)/internal/set_table.h

ractor.c

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2066,6 +2066,31 @@ ractor_move(VALUE obj)
20662066
}
20672067
}
20682068

2069+
static VALUE
2070+
ractor_call_clone_try(VALUE obj)
2071+
{
2072+
return rb_funcall(obj, idClone, 0);
2073+
}
2074+
2075+
static VALUE
2076+
ractor_call_clone_rescue(VALUE obj, VALUE exc)
2077+
{
2078+
rb_raise(rb_eRactorError, "can't clone unshareable instance of %"PRIsVALUE, rb_class_of(obj));
2079+
UNREACHABLE_RETURN(Qnil);
2080+
}
2081+
2082+
static VALUE
2083+
ractor_obj_clone(VALUE obj)
2084+
{
2085+
VALUE clone = rb_rescue(ractor_call_clone_try, obj, ractor_call_clone_rescue, obj);
2086+
2087+
if (obj == clone) {
2088+
rb_raise(rb_eRactorError, "#clone returned self");
2089+
}
2090+
2091+
return clone;
2092+
}
2093+
20692094
static enum obj_traverse_iterator_result
20702095
copy_enter(VALUE obj, struct obj_traverse_replace_data *data)
20712096
{
@@ -2074,10 +2099,7 @@ copy_enter(VALUE obj, struct obj_traverse_replace_data *data)
20742099
return traverse_skip;
20752100
}
20762101
else {
2077-
if (!rb_get_alloc_func(rb_obj_class(obj))) {
2078-
rb_raise(rb_eRactorError, "can not copy unshareable object %+"PRIsVALUE, obj);
2079-
}
2080-
data->replacement = rb_obj_clone(obj);
2102+
data->replacement = ractor_obj_clone(obj);
20812103
return traverse_cont;
20822104
}
20832105
}

template/id.c.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
%# -*- c -*-
2-
/* DO NOT EDIT THIS FILE DIRECTLY */
2+
/* DO NOT EDIT THIS FILE DIRECTLY: source is at template/id.c.tmpl */
33
/**********************************************************************
44
55
id.c -

test/ruby/test_ractor.rb

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,57 @@ def x.to_s
5050
assert_unshareable(x, "can not make shareable object for #<Method: String#to_s()> because it refers unshareable objects", exception: Ractor::Error)
5151
end
5252

53+
def test_sending_exception_with_backtrace
54+
assert_ractor(<<~'RUBY')
55+
def build_error
56+
raise "Test"
57+
rescue => error
58+
error
59+
end
60+
61+
error = build_error
62+
refute_empty error.backtrace
63+
refute_empty error.backtrace_locations
64+
65+
backtrace, backtrace_locations = Ractor.new(error) do |error2|
66+
[error2.backtrace, error2.backtrace_locations]
67+
end.value
68+
69+
assert_equal error.backtrace, backtrace
70+
refute_empty backtrace_locations
71+
RUBY
72+
end
73+
74+
def test_sending_exception_with_array_backtrace
75+
assert_ractor(<<~'RUBY')
76+
error = StandardError.new
77+
error.set_backtrace(["foo", "bar"])
78+
refute_empty error.backtrace
79+
assert_nil error.backtrace_locations
80+
81+
backtrace, backtrace_locations = Ractor.new(error) do |error2|
82+
[error2.backtrace, error2.backtrace_locations]
83+
end.value
84+
85+
assert_equal error.backtrace, backtrace
86+
assert_nil backtrace_locations
87+
RUBY
88+
end
89+
90+
def test_sending_object_with_broken_clone
91+
assert_ractor(<<~'RUBY')
92+
o = Object.new
93+
def o.clone
94+
self
95+
end
96+
ractor = Ractor.new { Ractor.receive }
97+
error = assert_raise Ractor::Error do
98+
ractor.send(o)
99+
end
100+
assert_match "#clone returned self", error.message
101+
RUBY
102+
end
103+
53104
def test_default_thread_group
54105
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
55106
begin;
@@ -219,7 +270,7 @@ def test_copy_unshareable_object_error_message
219270
err = assert_raise(Ractor::Error) do
220271
Ractor.new(pr) {}.join
221272
end
222-
assert_match(/can not copy unshareable object/, err.message)
273+
assert_match(/can not copy Proc object/, err.message)
223274
RUBY
224275
end
225276

vm_backtrace.c

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "internal.h"
1414
#include "internal/class.h"
1515
#include "internal/error.h"
16+
#include "internal/object.h"
1617
#include "internal/vm.h"
1718
#include "iseq.h"
1819
#include "ruby/debug.h"
@@ -576,14 +577,6 @@ rb_backtrace_p(VALUE obj)
576577
return rb_typeddata_is_kind_of(obj, &backtrace_data_type);
577578
}
578579

579-
static VALUE
580-
backtrace_alloc(VALUE klass)
581-
{
582-
rb_backtrace_t *bt;
583-
VALUE obj = TypedData_Make_Struct(klass, rb_backtrace_t, &backtrace_data_type, bt);
584-
return obj;
585-
}
586-
587580
static VALUE
588581
backtrace_alloc_capa(long num_frames, rb_backtrace_t **backtrace)
589582
{
@@ -962,6 +955,46 @@ backtrace_limit(VALUE self)
962955
return LONG2NUM(rb_backtrace_length_limit);
963956
}
964957

958+
/* :nodoc: */
959+
static VALUE
960+
backtrace_clone(VALUE self)
961+
{
962+
rb_backtrace_t *bt;
963+
TypedData_Get_Struct(self, rb_backtrace_t, &backtrace_data_type, bt);
964+
965+
rb_backtrace_t *other_bt;
966+
VALUE clone = backtrace_alloc_capa(bt->backtrace_size, &other_bt);
967+
968+
rb_obj_clone_setup(self, clone, Qfalse);
969+
970+
return clone;
971+
}
972+
973+
/* :nodoc: */
974+
static VALUE
975+
backtrace_dup(VALUE self)
976+
{
977+
rb_notimplement();
978+
979+
UNREACHABLE_RETURN(Qnil);
980+
}
981+
982+
/* :nodoc: */
983+
static VALUE
984+
backtrace_initialize_copy(VALUE self, VALUE original)
985+
{
986+
rb_backtrace_t *bt;
987+
TypedData_Get_Struct(self, rb_backtrace_t, &backtrace_data_type, bt);
988+
989+
rb_backtrace_t *original_bt;
990+
TypedData_Get_Struct(original, rb_backtrace_t, &backtrace_data_type, original_bt);
991+
992+
bt->backtrace_size = original_bt->backtrace_size;
993+
MEMCPY(bt->backtrace, original_bt->backtrace, rb_backtrace_location_t, original_bt->backtrace_size);
994+
995+
return Qnil;
996+
}
997+
965998
VALUE
966999
rb_ec_backtrace_str_ary(const rb_execution_context_t *ec, long lev, long n)
9671000
{
@@ -1406,6 +1439,14 @@ each_caller_location(int argc, VALUE *argv, VALUE _)
14061439
return Qnil;
14071440
}
14081441

1442+
static VALUE
1443+
backtrace_no_allocator(VALUE klass)
1444+
{
1445+
rb_notimplement();
1446+
1447+
UNREACHABLE_RETURN(Qnil);
1448+
}
1449+
14091450
/* called from Init_vm() in vm.c */
14101451
void
14111452
Init_vm_backtrace(void)
@@ -1416,11 +1457,18 @@ Init_vm_backtrace(void)
14161457
* settings of the current session.
14171458
*/
14181459
rb_cBacktrace = rb_define_class_under(rb_cThread, "Backtrace", rb_cObject);
1419-
rb_define_alloc_func(rb_cBacktrace, backtrace_alloc);
1420-
rb_undef_method(CLASS_OF(rb_cBacktrace), "new");
1460+
1461+
// Can't undefine the allocator, as it's needed as a key by Marshal
1462+
rb_define_alloc_func(rb_cBacktrace, backtrace_no_allocator);
14211463
rb_marshal_define_compat(rb_cBacktrace, rb_cArray, backtrace_dump_data, backtrace_load_data);
1464+
1465+
rb_undef_method(CLASS_OF(rb_cBacktrace), "new");
14221466
rb_define_singleton_method(rb_cBacktrace, "limit", backtrace_limit, 0);
14231467

1468+
rb_define_method(rb_cBacktrace, "clone", backtrace_clone, 0);
1469+
rb_define_method(rb_cBacktrace, "dup", backtrace_dup, 0);
1470+
rb_define_method(rb_cBacktrace, "initialize_copy", backtrace_initialize_copy, 1);
1471+
14241472
/*
14251473
* An object representation of a stack frame, initialized by
14261474
* Kernel#caller_locations.

0 commit comments

Comments
 (0)