Skip to content

Commit 499d6ae

Browse files
committed
Singleton classes of unshareable objects are not shareable
Currently, all RClasses are created as shareable in `class_alloc0`, which causes singleton classes of unshareable objects to be shareable which allows passing unshareable objects by passing their singleton class and getting their attached object. This changes `singleton_class_of` to mark the singleton class as shareable when the object is shareable, and inversely, to mark the singleton class as unshareable if the object is unshareable. We also need to not forcibly mark the class as shareable in `shareable_p_enter`. Additionally, if the object is not yet marked as shareable, but is frozen (and `allow_frozen_shareable_p`), it becomes marked as shareable through `rb_ractor_shareable_p`, and we need to also mark its singleton class as shareable at that point. This also requires changing GC.verify_internal_consistency to allow these shareable-to-unshareable references: - method entries to their owner and definition classes - iclass to their class - fields to their class This is sound since these objects can't be accessed from Ruby.
1 parent 2daf48e commit 499d6ae

6 files changed

Lines changed: 53 additions & 9 deletions

File tree

class.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "internal/string.h"
3030
#include "internal/variable.h"
3131
#include "ruby/st.h"
32+
#include "ruby/ractor.h"
3233
#include "vm_core.h"
3334
#include "ruby/ractor.h"
3435
#include "yjit.h"
@@ -2871,6 +2872,12 @@ singleton_class_of(VALUE obj, bool ensure_eigenclass)
28712872
klass = rb_make_metaclass(obj, klass);
28722873
}
28732874
RB_FL_SET_RAW(klass, RB_OBJ_FROZEN_RAW(obj));
2875+
if (!RB_OBJ_SHAREABLE_P(obj) && RB_OBJ_SHAREABLE_P(klass)) {
2876+
RB_FL_UNSET_RAW(klass, RUBY_FL_SHAREABLE);
2877+
}
2878+
else if (RB_OBJ_SHAREABLE_P(obj) && !RB_OBJ_SHAREABLE_P(klass)) {
2879+
RB_FL_SET_RAW(klass, RUBY_FL_SHAREABLE);
2880+
}
28742881
if (ensure_eigenclass && RB_TYPE_P(obj, T_CLASS)) {
28752882
/* ensures an exposed class belongs to its own eigenclass */
28762883
(void)ENSURE_EIGENCLASS(klass);

depend

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12357,6 +12357,7 @@ ractor.$(OBJEXT): $(top_srcdir)/internal/basic_operators.h
1235712357
ractor.$(OBJEXT): $(top_srcdir)/internal/bignum.h
1235812358
ractor.$(OBJEXT): $(top_srcdir)/internal/bits.h
1235912359
ractor.$(OBJEXT): $(top_srcdir)/internal/box.h
12360+
ractor.$(OBJEXT): $(top_srcdir)/internal/class.h
1236012361
ractor.$(OBJEXT): $(top_srcdir)/internal/compilers.h
1236112362
ractor.$(OBJEXT): $(top_srcdir)/internal/complex.h
1236212363
ractor.$(OBJEXT): $(top_srcdir)/internal/error.h

gc.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3278,7 +3278,11 @@ gc_mark_classext_iclass(rb_classext_t *ext, bool prime, VALUE box_value, void *a
32783278
mark_m_tbl(objspace, RCLASSEXT_M_TBL(ext));
32793279
}
32803280
if (RCLASSEXT_INCLUDER(ext)) {
3281-
gc_mark_internal(RCLASSEXT_INCLUDER(ext));
3281+
if (LIKELY(!rb_gc_checking_shareable()) ||
3282+
!RB_TYPE_P(RCLASSEXT_INCLUDER(ext), T_CLASS) || !FL_TEST_RAW(RCLASSEXT_INCLUDER(ext), FL_SINGLETON)) {
3283+
// module can be included in an unshareable singleton class
3284+
gc_mark_internal(RCLASSEXT_INCLUDER(ext));
3285+
}
32823286
}
32833287
mark_m_tbl(objspace, RCLASSEXT_CALLABLE_M_TBL(ext));
32843288
gc_mark_internal(RCLASSEXT_CC_TBL(ext));

imemo.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,16 @@ mark_and_move_method_entry(rb_method_entry_t *ment, bool reference_updating)
296296
{
297297
rb_method_definition_t *def = ment->def;
298298

299-
rb_gc_mark_and_move(&ment->owner);
300-
rb_gc_mark_and_move(&ment->defined_class);
299+
if (LIKELY(!rb_gc_checking_shareable()) ||
300+
!RB_TYPE_P(ment->owner, T_CLASS) || !FL_TEST_RAW(ment->owner, FL_SINGLETON)) {
301+
// method entry can be owned by a singleton unshareable class
302+
rb_gc_mark_and_move(&ment->owner);
303+
}
304+
if (LIKELY(!rb_gc_checking_shareable()) ||
305+
!RB_TYPE_P(ment->defined_class, T_CLASS) || !FL_TEST_RAW(ment->defined_class, FL_SINGLETON)) {
306+
// method entry can be defined in a singleton unshareable class
307+
rb_gc_mark_and_move(&ment->defined_class);
308+
}
301309

302310
if (def) {
303311
switch (def->type) {
@@ -512,11 +520,10 @@ rb_imemo_mark_and_move(VALUE obj, bool reference_updating)
512520
break;
513521
}
514522
case imemo_fields: {
515-
rb_gc_mark_and_move((VALUE *)&RBASIC(obj)->klass);
516-
517-
if (!rb_gc_checking_shareable()) {
523+
if (LIKELY(!rb_gc_checking_shareable())) {
518524
// imemo_fields can refer unshareable objects
519525
// even if the imemo_fields is shareable.
526+
rb_gc_mark_and_move((VALUE *)&RBASIC(obj)->klass);
520527

521528
if (rb_shape_obj_too_complex_p(obj)) {
522529
st_table *tbl = rb_imemo_fields_complex_tbl(obj);

ractor.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "vm_core.h"
99
#include "vm_sync.h"
1010
#include "ractor_core.h"
11+
#include "internal/class.h"
1112
#include "internal/complex.h"
1213
#include "internal/error.h"
1314
#include "internal/gc.h"
@@ -1545,6 +1546,9 @@ mark_shareable(VALUE obj)
15451546
}
15461547

15471548
rb_obj_set_shareable_no_assert(obj);
1549+
if (RCLASS_SINGLETON_P(RBASIC(obj)->klass)) {
1550+
rb_obj_set_shareable_no_assert(RBASIC(obj)->klass);
1551+
}
15481552
return traverse_cont;
15491553
}
15501554

@@ -1592,9 +1596,14 @@ shareable_p_enter(VALUE obj)
15921596
else if (RB_TYPE_P(obj, T_CLASS) ||
15931597
RB_TYPE_P(obj, T_MODULE) ||
15941598
RB_TYPE_P(obj, T_ICLASS)) {
1595-
// TODO: remove it
1596-
mark_shareable(obj);
1597-
return traverse_skip;
1599+
if (RCLASS_SINGLETON_P(obj) && !RB_OBJ_SHAREABLE_P(RCLASS_ATTACHED_OBJECT(obj))) {
1600+
return traverse_stop;
1601+
}
1602+
else {
1603+
// TODO: remove it
1604+
mark_shareable(obj);
1605+
return traverse_skip;
1606+
}
15981607
}
15991608
else if (RB_OBJ_FROZEN_RAW(obj) &&
16001609
allow_frozen_shareable_p(obj)) {

test/ruby/test_ractor.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,22 @@ def test_ractor_new_raises_isolation_error_if_proc_uses_yield
231231
end
232232
end
233233

234+
def test_singleton_class_of_unshareable_object_is_unshareable
235+
obj = Object.new
236+
obj.instance_variable_set(:@unshareable, ->{})
237+
refute Ractor.shareable?(obj.freeze), "object with a unshareable Proc is shareable"
238+
refute Ractor.shareable?(obj.singleton_class), "singleton class of unshareable object is shareable"
239+
GC.verify_internal_consistency
240+
end
241+
242+
def test_module_can_be_included_in_unshareable_object
243+
obj = Object.new
244+
obj.instance_variable_set(:@unshareable, ->{})
245+
obj.extend(Module.new)
246+
refute Ractor.shareable?(obj.freeze), "object with a unshareable Proc is shareable"
247+
GC.verify_internal_consistency
248+
end
249+
234250
def assert_make_shareable(obj)
235251
refute Ractor.shareable?(obj), "object was already shareable"
236252
Ractor.make_shareable(obj)

0 commit comments

Comments
 (0)