Skip to content

Commit d7a36b4

Browse files
committed
Ractor: Fix moving embedded objects
`rb_obj_alloc(RBASIC_CLASS(obj))` will always allocate from the basic 40B pool, so if `obj` is larger than `40B`, we'll create a corrupted object when we later copy the shape_id. Instead we can use the same logic than ractor copy, which is to use `rb_obj_clone`, and later ask the GC to free the original object. We then must turn it into a `T_OBJECT`, because otherwise just changing its class to `RactorMoved` leaves a lot of ways to keep using the object, e.g.: ``` a = [1, 2, 3] Ractor.new{}.send(a, move: true) [].concat(a) # Should raise, but wasn't. ``` If it turns out that `rb_obj_clone` isn't performant enough for some uses, we can always have carefully crafted specialized paths for the types that would benefit from it.
1 parent fc26004 commit d7a36b4

File tree

5 files changed

+169
-78
lines changed

5 files changed

+169
-78
lines changed

bootstraptest/test_ractor.rb

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1987,3 +1987,141 @@ def require feature
19871987
GC.start
19881988
:ok.itself
19891989
}
1990+
1991+
# moved objects being corrupted if embeded (String)
1992+
assert_equal 'ok', %q{
1993+
ractor = Ractor.new do
1994+
Ractor.receive
1995+
end
1996+
obj = "foobarbazfoobarbazfoobarbazfoobarbaz"
1997+
ractor.send(obj.dup, move: true)
1998+
rountriped_obj = ractor.take
1999+
rountriped_obj == obj ? :ok : rountriped_obj
2000+
}
2001+
2002+
# moved objects being corrupted if embeded (Array)
2003+
assert_equal 'ok', %q{
2004+
ractor = Ractor.new do
2005+
Ractor.receive
2006+
end
2007+
obj = Array.new(10, 42)
2008+
ractor.send(obj.dup, move: true)
2009+
rountriped_obj = ractor.take
2010+
rountriped_obj == obj ? :ok : rountriped_obj
2011+
}
2012+
2013+
# moved objects being corrupted if embeded (Hash)
2014+
assert_equal 'ok', %q{
2015+
ractor = Ractor.new do
2016+
Ractor.receive
2017+
end
2018+
obj = { foo: 1, bar: 2 }
2019+
ractor.send(obj.dup, move: true)
2020+
rountriped_obj = ractor.take
2021+
rountriped_obj == obj ? :ok : rountriped_obj
2022+
}
2023+
2024+
# moved objects being corrupted if embeded (MatchData)
2025+
assert_equal 'ok', %q{
2026+
ractor = Ractor.new do
2027+
Ractor.receive
2028+
end
2029+
obj = "foo".match(/o/)
2030+
ractor.send(obj.dup, move: true)
2031+
rountriped_obj = ractor.take
2032+
rountriped_obj == obj ? :ok : rountriped_obj
2033+
}
2034+
2035+
# moved objects being corrupted if embeded (Struct)
2036+
assert_equal 'ok', %q{
2037+
ractor = Ractor.new do
2038+
Ractor.receive
2039+
end
2040+
obj = Struct.new(:a, :b, :c, :d, :e, :f).new(1, 2, 3, 4, 5, 6)
2041+
ractor.send(obj.dup, move: true)
2042+
rountriped_obj = ractor.take
2043+
rountriped_obj == obj ? :ok : rountriped_obj
2044+
}
2045+
2046+
# moved objects being corrupted if embeded (Object)
2047+
assert_equal 'ok', %q{
2048+
ractor = Ractor.new do
2049+
Ractor.receive
2050+
end
2051+
class SomeObject
2052+
attr_reader :a, :b, :c, :d, :e, :f
2053+
def initialize
2054+
@a = @b = @c = @d = @e = @f = 1
2055+
end
2056+
2057+
def ==(o)
2058+
@a == o.a &&
2059+
@b == o.b &&
2060+
@c == o.c &&
2061+
@d == o.d &&
2062+
@e == o.e &&
2063+
@f == o.f
2064+
end
2065+
end
2066+
2067+
SomeObject.new # initial non-embeded
2068+
2069+
obj = SomeObject.new
2070+
ractor.send(obj.dup, move: true)
2071+
rountriped_obj = ractor.take
2072+
rountriped_obj == obj ? :ok : rountriped_obj
2073+
}
2074+
2075+
# moved arrays can't be used
2076+
assert_equal 'ok', %q{
2077+
ractor = Ractor.new {}
2078+
obj = [1]
2079+
ractor.send(obj, move: true)
2080+
begin
2081+
[].concat(obj)
2082+
rescue TypeError
2083+
:ok
2084+
else
2085+
:fail
2086+
end
2087+
}
2088+
2089+
# moved strings can't be used
2090+
assert_equal 'ok', %q{
2091+
ractor = Ractor.new {}
2092+
obj = "hello"
2093+
ractor.send(obj, move: true)
2094+
begin
2095+
"".replace(obj)
2096+
rescue TypeError
2097+
:ok
2098+
else
2099+
:fail
2100+
end
2101+
}
2102+
2103+
# moved hashes can't be used
2104+
assert_equal 'ok', %q{
2105+
ractor = Ractor.new {}
2106+
obj = { a: 1 }
2107+
ractor.send(obj, move: true)
2108+
begin
2109+
{}.merge(obj)
2110+
rescue TypeError
2111+
:ok
2112+
else
2113+
:fail
2114+
end
2115+
}
2116+
2117+
# moved objects keep their object_id
2118+
assert_equal 'ok', %q{
2119+
ractor = Ractor.new do
2120+
Ractor.receive
2121+
end
2122+
obj = Object.new
2123+
id = obj.object_id
2124+
ractor.send(obj, move: true)
2125+
rountriped_obj = ractor.take
2126+
rountriped_obj.object_id == id ? :ok : :fail
2127+
}

gc.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2659,6 +2659,32 @@ rb_gc_mark_roots(void *objspace, const char **categoryp)
26592659

26602660
#define TYPED_DATA_REFS_OFFSET_LIST(d) (size_t *)(uintptr_t)RTYPEDDATA(d)->type->function.dmark
26612661

2662+
void
2663+
rb_gc_ractor_moved(VALUE dest, VALUE src)
2664+
{
2665+
st_data_t id = 0;
2666+
if (UNLIKELY(FL_TEST_RAW(src, FL_SEEN_OBJ_ID))) {
2667+
/* If the source object's object_id has been seen, we need to update
2668+
* the object to object id mapping. */
2669+
rb_objspace_t *objspace = rb_gc_get_objspace();
2670+
2671+
unsigned int lev = rb_gc_vm_lock();
2672+
if (!st_lookup(objspace->obj_to_id_tbl, (st_data_t)src, &id)) {
2673+
rb_bug("gc_move: object ID seen, but not in mapping table: %s", rb_obj_info(src));
2674+
}
2675+
st_insert(objspace->obj_to_id_tbl, (st_data_t)dest, id);
2676+
rb_gc_vm_unlock(lev);
2677+
// rb_gc_obj_free will take care of deleting the src entry
2678+
}
2679+
2680+
rb_gc_obj_free(rb_gc_get_objspace(), src);
2681+
MEMZERO((void *)src, char, rb_gc_obj_slot_size(src));
2682+
RBASIC(src)->flags = T_OBJECT | FL_FREEZE; // Avoid mutations using bind_call, etc.
2683+
if (id) {
2684+
FL_SET_RAW(dest, FL_SEEN_OBJ_ID);
2685+
}
2686+
}
2687+
26622688
void
26632689
rb_gc_mark_children(void *objspace, VALUE obj)
26642690
{

internal/gc.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ struct rb_gc_object_metadata_entry {
183183
/* gc.c */
184184
RUBY_ATTR_MALLOC void *ruby_mimmalloc(size_t size);
185185
RUBY_ATTR_MALLOC void *ruby_mimcalloc(size_t num, size_t size);
186+
void rb_gc_ractor_moved(VALUE dest, VALUE src);
186187
void ruby_mimfree(void *ptr);
187188
void rb_gc_prepare_heap(void);
188189
void rb_objspace_set_event_hook(const rb_event_flag_t event);

ractor.c

Lines changed: 4 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "internal/error.h"
1313
#include "internal/gc.h"
1414
#include "internal/hash.h"
15+
#include "internal/object.h"
1516
#include "internal/ractor.h"
1617
#include "internal/rational.h"
1718
#include "internal/struct.h"
@@ -3546,37 +3547,6 @@ rb_obj_traverse_replace(VALUE obj,
35463547
}
35473548
}
35483549

3549-
struct RVALUE {
3550-
VALUE flags;
3551-
VALUE klass;
3552-
VALUE v1;
3553-
VALUE v2;
3554-
VALUE v3;
3555-
};
3556-
3557-
static const VALUE fl_users = FL_USER1 | FL_USER2 | FL_USER3 |
3558-
FL_USER4 | FL_USER5 | FL_USER6 | FL_USER7 |
3559-
FL_USER8 | FL_USER9 | FL_USER10 | FL_USER11 |
3560-
FL_USER12 | FL_USER13 | FL_USER14 | FL_USER15 |
3561-
FL_USER16 | FL_USER17 | FL_USER18 | FL_USER19;
3562-
3563-
static void
3564-
ractor_moved_bang(VALUE obj)
3565-
{
3566-
// invalidate src object
3567-
struct RVALUE *rv = (void *)obj;
3568-
3569-
rv->klass = rb_cRactorMovedObject;
3570-
rv->v1 = 0;
3571-
rv->v2 = 0;
3572-
rv->v3 = 0;
3573-
rv->flags = rv->flags & ~fl_users;
3574-
3575-
if (BUILTIN_TYPE(obj) == T_OBJECT) ROBJECT_SET_SHAPE_ID(obj, ROOT_SHAPE_ID);
3576-
3577-
// TODO: record moved location
3578-
}
3579-
35803550
static enum obj_traverse_iterator_result
35813551
move_enter(VALUE obj, struct obj_traverse_replace_data *data)
35823552
{
@@ -3585,39 +3555,16 @@ move_enter(VALUE obj, struct obj_traverse_replace_data *data)
35853555
return traverse_skip;
35863556
}
35873557
else {
3588-
VALUE moved = rb_obj_alloc(RBASIC_CLASS(obj));
3589-
rb_shape_set_shape(moved, rb_shape_get_shape(obj));
3590-
data->replacement = moved;
3558+
data->replacement = rb_obj_clone(obj);
35913559
return traverse_cont;
35923560
}
35933561
}
35943562

3595-
void rb_replace_generic_ivar(VALUE clone, VALUE obj); // variable.c
3596-
35973563
static enum obj_traverse_iterator_result
35983564
move_leave(VALUE obj, struct obj_traverse_replace_data *data)
35993565
{
3600-
VALUE v = data->replacement;
3601-
struct RVALUE *dst = (struct RVALUE *)v;
3602-
struct RVALUE *src = (struct RVALUE *)obj;
3603-
3604-
dst->flags = (dst->flags & ~fl_users) | (src->flags & fl_users);
3605-
3606-
dst->v1 = src->v1;
3607-
dst->v2 = src->v2;
3608-
dst->v3 = src->v3;
3609-
3610-
if (UNLIKELY(FL_TEST_RAW(obj, FL_EXIVAR))) {
3611-
rb_replace_generic_ivar(v, obj);
3612-
}
3613-
3614-
if (OBJ_FROZEN(obj)) {
3615-
OBJ_FREEZE(v);
3616-
}
3617-
3618-
// TODO: generic_ivar
3619-
3620-
ractor_moved_bang(obj);
3566+
rb_gc_ractor_moved(data->replacement, obj);
3567+
RBASIC_SET_CLASS_RAW(obj, rb_cRactorMovedObject);
36213568
return traverse_cont;
36223569
}
36233570

variable.c

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2157,27 +2157,6 @@ rb_copy_generic_ivar(VALUE clone, VALUE obj)
21572157
}
21582158
}
21592159

2160-
void
2161-
rb_replace_generic_ivar(VALUE clone, VALUE obj)
2162-
{
2163-
RUBY_ASSERT(FL_TEST(obj, FL_EXIVAR));
2164-
2165-
RB_VM_LOCK_ENTER();
2166-
{
2167-
st_data_t ivtbl, obj_data = (st_data_t)obj;
2168-
if (st_lookup(generic_iv_tbl_, (st_data_t)obj, &ivtbl)) {
2169-
st_insert(generic_iv_tbl_, (st_data_t)clone, ivtbl);
2170-
st_delete(generic_iv_tbl_, &obj_data, NULL);
2171-
}
2172-
else {
2173-
rb_bug("unreachable");
2174-
}
2175-
}
2176-
RB_VM_LOCK_LEAVE();
2177-
2178-
FL_SET(clone, FL_EXIVAR);
2179-
}
2180-
21812160
void
21822161
rb_ivar_foreach(VALUE obj, rb_ivar_foreach_callback_func *func, st_data_t arg)
21832162
{

0 commit comments

Comments
 (0)