Skip to content

Commit c7c0483

Browse files
ianksjhawthorn
authored andcommitted
ractor.c: preserve aliasing during Ractor move
Moving an unshareable object graph with duplicate references failed to preserve aliasing because the traversal checked the `rec` table too late. On a move path, `move_leave` replaces the original object with a `Ractor::MovedObject` tombstone. Because `move_enter` classifies this tombstone as shareable, the second visit returned `traverse_skip` before the visited `rec` lookup was reached. Fix this by performing the `st_lookup` on `data->rec` at the top of `obj_traverse_replace_i`, before calling `enter_func`. We guard the lookup with a NULL check on `data->rec` to avoid allocating `rec_hash` for simple graphs.
1 parent faa3cc8 commit c7c0483

2 files changed

Lines changed: 31 additions & 10 deletions

File tree

bootstraptest/test_ractor.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,29 @@ def check obj1
656656
end
657657
}
658658

659+
# move preserves aliasing inside the moved object graph
660+
assert_equal 'true', %q{
661+
r = Ractor.new do
662+
Ractor.receive
663+
end
664+
665+
leaf = +"leaf"
666+
moved = r.send([leaf, leaf], move: true).value
667+
moved[0].equal?(moved[1])
668+
}
669+
670+
# move handles cyclic references safely and preserves aliasing
671+
assert_equal 'true', %q{
672+
r = Ractor.new do
673+
Ractor.receive
674+
end
675+
676+
a = []
677+
a << a
678+
moved = r.send(a, move: true).value
679+
moved.equal?(moved[0])
680+
}
681+
659682
# unshareable frozen objects should still be frozen in new ractor after move
660683
assert_equal 'true', %q{
661684
r = Ractor.new do

ractor.c

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1841,23 +1841,21 @@ obj_traverse_replace_i(VALUE obj, struct obj_traverse_replace_data *data)
18411841
return 0;
18421842
}
18431843

1844+
if (UNLIKELY(data->rec && st_lookup(data->rec, (st_data_t)obj, &replacement))) {
1845+
data->replacement = (VALUE)replacement;
1846+
return 0;
1847+
}
1848+
18441849
switch (data->enter_func(obj, data)) {
18451850
case traverse_cont: break;
18461851
case traverse_skip: return 0; // skip children
18471852
case traverse_stop: return 1; // stop search
18481853
}
18491854

18501855
replacement = (st_data_t)data->replacement;
1851-
1852-
if (UNLIKELY(st_lookup(obj_traverse_replace_rec(data), (st_data_t)obj, &replacement))) {
1853-
data->replacement = (VALUE)replacement;
1854-
return 0;
1855-
}
1856-
else {
1857-
st_insert(obj_traverse_replace_rec(data), (st_data_t)obj, replacement);
1858-
RB_OBJ_WRITTEN(data->rec_hash, Qundef, obj);
1859-
RB_OBJ_WRITTEN(data->rec_hash, Qundef, replacement);
1860-
}
1856+
st_insert(obj_traverse_replace_rec(data), (st_data_t)obj, replacement);
1857+
RB_OBJ_WRITTEN(data->rec_hash, Qundef, obj);
1858+
RB_OBJ_WRITTEN(data->rec_hash, Qundef, replacement);
18611859

18621860
if (!data->move) {
18631861
obj = replacement;

0 commit comments

Comments
 (0)