Fix a race condition in object_id for shareable objects

If an object is shareable and has no capacity left, it isn't
safe to store the object ID in fields as it requires an object
resize which can't be done unless all field reads are synchronized.

In this very specific case we create the object_id in advance,
before the object is made shareable.
This commit is contained in:
Jean Boussier 2025-06-13 14:25:42 +02:00
parent 0674f7dfb5
commit e22fc73c66
Notes: git 2025-06-13 16:28:09 +00:00
2 changed files with 97 additions and 3 deletions

View File

@ -1357,9 +1357,26 @@ make_shareable_check_shareable(VALUE obj)
}
}
if (RB_TYPE_P(obj, T_IMEMO)) {
return traverse_skip;
}
switch (TYPE(obj)) {
case T_IMEMO:
return traverse_skip;
case T_OBJECT:
{
// If a T_OBJECT is shared and has no free capacity, we can't safely store the object_id inline,
// as it would require to move the object content into an external buffer.
// This is only a problem for T_OBJECT, given other types have external fields and can do RCU.
// To avoid this issue, we proactively create the object_id.
shape_id_t shape_id = RBASIC_SHAPE_ID(obj);
attr_index_t capacity = RSHAPE_CAPACITY(shape_id);
attr_index_t free_capacity = capacity - RSHAPE_LEN(shape_id);
if (!rb_shape_has_object_id(shape_id) && capacity && !free_capacity) {
rb_obj_id(obj);
}
}
break;
default:
break;
}
if (!RB_OBJ_FROZEN_RAW(obj)) {
rb_funcall(obj, idFreeze, 0);

View File

@ -198,3 +198,80 @@ class TestObjectIdTooComplexGeneric < TestObjectId
end
end
end
class TestObjectIdRactor < Test::Unit::TestCase
def test_object_id_race_free
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
begin;
Warning[:experimental] = false
class MyClass
attr_reader :a, :b, :c
def initialize
@a = @b = @c = nil
end
end
N = 10_000
objs = Ractor.make_shareable(N.times.map { MyClass.new })
results = 4.times.map{
Ractor.new(objs) { |objs|
vars = []
ids = []
objs.each do |obj|
vars << obj.a << obj.b << obj.c
ids << obj.object_id
end
[vars, ids]
}
}.map(&:value)
assert_equal 1, results.uniq.size
end;
end
def test_external_object_id_ractor_move
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
begin;
Warning[:experimental] = false
class MyClass
attr_reader :a, :b, :c
def initialize
@a = @b = @c = nil
end
end
obj = Ractor.make_shareable(MyClass.new)
object_id = obj.object_id
obj = Ractor.new { Ractor.receive }.send(obj, move: true).value
assert_equal object_id, obj.object_id
end;
end
def test_object_id_race_free_with_stress_compact
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
begin;
Warning[:experimental] = false
class MyClass
attr_reader :a, :b, :c
def initialize
@a = @b = @c = nil
end
end
N = 20
objs = Ractor.make_shareable(N.times.map { MyClass.new })
GC.stress = true
GC.auto_compact = true if GC.respond_to?(:auto_compact=)
results = 4.times.map{
Ractor.new(objs) { |objs|
vars = []
ids = []
objs.each do |obj|
vars << obj.a << obj.b << obj.c
ids << obj.object_id
end
[vars, ids]
}
}.map(&:value)
assert_equal 1, results.uniq.size
end;
end
end