Skip to content

Ractor: Fix moving embedded objects #12997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

casperisfine
Copy link
Contributor

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.

I chose to use specialized rb_*_dup functions when possible to benefit from shared String/Arrays and trigger write barriers.

For some other types I resorted to the generic rb_obj_dup. It works but probably isn't ideal.

@casperisfine casperisfine changed the title Ractor: Fix moving embeded objects Ractor: Fix moving embedded objects Mar 27, 2025
@@ -3585,8 +3585,27 @@ move_enter(VALUE obj, struct obj_traverse_replace_data *data)
return traverse_skip;
}
else {
VALUE moved = rb_obj_alloc(RBASIC_CLASS(obj));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this was preserving the metaclass so perhaps using dup is incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, the previous behavior wasn't particularly useful:

>> o = Object.new
=> #<Object:0x000000012b396a30>
>> def o.foo = 12
=> :foo
>> Ractor.new { Ractor.receive }.send(o, move: true).take
(irb):3: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues.
<internal:ractor>:600:in 'Ractor#send': can't create instance of singleton class (TypeError)

@byroot byroot requested review from tenderlove, ko1 and jhawthorn March 27, 2025 13:54
@casperisfine
Copy link
Contributor Author

@jhawthorn pointed that move_leave also assume no WVA, so it needs to be updated too.

byroot added 2 commits March 28, 2025 12:53
`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.

I chose to use specialized `rb_*_dup` functions when possible to
benefit from shared String/Arrays and trigger write barriers.

For some other types I resorted to the generic `rb_obj_dup`.
It works but probably isn't ideal.
Copy link

launchable-app bot commented Mar 28, 2025

All Tests passed!

✖️no tests failed ✔️61729 tests passed(8 flakes)

@casperisfine
Copy link
Contributor Author

Closing in favor of: #13008

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants