Skip to content

Commit bf7de73

Browse files
byrootbyroot
committed
gc.c: 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. So in this case we have to store the ID externally like we used to.
1 parent 5ec9a39 commit bf7de73

5 files changed

Lines changed: 199 additions & 35 deletions

File tree

‎gc.c‎

Lines changed: 127 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1853,6 +1853,55 @@ static const rb_data_type_t id2ref_tbl_type = {
18531853

18541854
#define RUBY_ATOMIC_VALUE_LOAD(x) (VALUE)(RUBY_ATOMIC_PTR_LOAD(x))
18551855

1856+
static VALUE obj_to_id_value = 0;
1857+
static st_table *obj_to_id_tbl = NULL;
1858+
1859+
static void mark_hash_values(st_table *tbl);
1860+
1861+
static void
1862+
obj_to_id_tbl_mark(void *data)
1863+
{
1864+
st_table *table = (st_table *)data;
1865+
if (UNLIKELY(!RB_POSFIXABLE(LAST_OBJECT_ID()))) {
1866+
// It's very unlikely, but if enough object ids were generated, keys may be T_BIGNUM
1867+
mark_hash_values(table);
1868+
}
1869+
// We purposedly don't mark keys, as they are weak references.
1870+
// rb_gc_obj_free_vm_weak_references takes care of cleaning them up.
1871+
}
1872+
1873+
static size_t
1874+
obj_to_id_tbl_memsize(const void *data)
1875+
{
1876+
return rb_st_memsize(data);
1877+
}
1878+
1879+
static void
1880+
obj_to_id_tbl_compact(void *data)
1881+
{
1882+
st_table *table = (st_table *)data;
1883+
gc_update_table_refs(table);
1884+
}
1885+
1886+
static void
1887+
obj_to_id_tbl_free(void *data)
1888+
{
1889+
obj_to_id_tbl = NULL; // clear global ref
1890+
st_table *table = (st_table *)data;
1891+
st_free_table(table);
1892+
}
1893+
1894+
static const rb_data_type_t obj_to_id_tbl_type = {
1895+
.wrap_struct_name = "VM/obj_to_id_table",
1896+
.function = {
1897+
.dmark = obj_to_id_tbl_mark,
1898+
.dfree = obj_to_id_tbl_free,
1899+
.dsize = obj_to_id_tbl_memsize,
1900+
.dcompact = obj_to_id_tbl_compact,
1901+
},
1902+
.flags = RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_FREE_IMMEDIATELY
1903+
};
1904+
18561905
static VALUE
18571906
class_object_id(VALUE klass)
18581907
{
@@ -1876,11 +1925,23 @@ static inline VALUE
18761925
object_id_get(VALUE obj, shape_id_t shape_id)
18771926
{
18781927
VALUE id;
1879-
if (rb_shape_too_complex_p(shape_id)) {
1880-
id = rb_obj_field_get(obj, ROOT_TOO_COMPLEX_WITH_OBJ_ID);
1881-
}
1882-
else {
1883-
id = rb_obj_field_get(obj, rb_shape_object_id(shape_id));
1928+
1929+
switch (rb_shape_has_object_id(shape_id)) {
1930+
case SHAPE_NO_OBJ_ID:
1931+
rb_bug("Unreacheable");
1932+
break;
1933+
case SHAPE_INLINE_OBJ_ID:
1934+
if (rb_shape_too_complex_p(shape_id)) {
1935+
id = rb_obj_field_get(obj, ROOT_TOO_COMPLEX_WITH_OBJ_ID);
1936+
}
1937+
else {
1938+
id = rb_obj_field_get(obj, rb_shape_object_id_inline(shape_id));
1939+
}
1940+
break;
1941+
case SHAPE_EXTERNAL_OBJ_ID:
1942+
RUBY_ASSERT(obj_to_id_tbl);
1943+
st_lookup(obj_to_id_tbl, obj, &id);
1944+
break;
18841945
}
18851946

18861947
#if RUBY_DEBUG
@@ -1894,7 +1955,7 @@ object_id_get(VALUE obj, shape_id_t shape_id)
18941955
}
18951956

18961957
static VALUE
1897-
object_id0(VALUE obj)
1958+
object_id0(VALUE obj, bool shareable)
18981959
{
18991960
VALUE id = Qfalse;
19001961
shape_id_t shape_id = RBASIC_SHAPE_ID(obj);
@@ -1903,14 +1964,26 @@ object_id0(VALUE obj)
19031964
return object_id_get(obj, shape_id);
19041965
}
19051966

1906-
// rb_shape_object_id_shape may lock if the current shape has
1907-
// multiple children.
1908-
shape_id_t object_id_shape_id = rb_shape_transition_object_id(obj);
1909-
19101967
id = generate_next_object_id();
1911-
rb_obj_field_set(obj, object_id_shape_id, id);
19121968

1913-
RUBY_ASSERT(RBASIC_SHAPE_ID(obj) == object_id_shape_id);
1969+
attr_index_t capacity = RSHAPE_CAPACITY(shape_id);
1970+
attr_index_t free_capacity = capacity - RSHAPE_LEN(shape_id);
1971+
if (shareable && capacity && !free_capacity) {
1972+
// The object is shared and has no free capacity, we can't
1973+
// safely store the object_id inline.
1974+
shape_id_t next_shape_id = rb_shape_transition_object_id_external(obj);
1975+
if (RB_UNLIKELY(!obj_to_id_tbl)) {
1976+
obj_to_id_tbl = st_init_numtable();
1977+
obj_to_id_value = TypedData_Wrap_Struct(0, &obj_to_id_tbl_type, obj_to_id_tbl);
1978+
}
1979+
st_insert(obj_to_id_tbl, obj, id);
1980+
RBASIC_SET_SHAPE_ID(obj, next_shape_id);
1981+
}
1982+
else {
1983+
shape_id_t next_shape_id = rb_shape_transition_object_id_inline(obj);
1984+
rb_obj_field_set(obj, next_shape_id, id);
1985+
}
1986+
19141987
RUBY_ASSERT(rb_shape_obj_has_id(obj));
19151988

19161989
if (RB_UNLIKELY(id2ref_tbl)) {
@@ -1936,14 +2009,14 @@ object_id(VALUE obj)
19362009
break;
19372010
}
19382011

1939-
if (UNLIKELY(rb_gc_multi_ractor_p() && rb_ractor_shareable_p(obj))) {
2012+
if (UNLIKELY(rb_gc_multi_ractor_p() && RB_OBJ_SHAREABLE_P(obj))) {
19402013
unsigned int lock_lev = RB_GC_VM_LOCK();
1941-
VALUE id = object_id0(obj);
2014+
VALUE id = object_id0(obj, true);
19422015
RB_GC_VM_UNLOCK(lock_lev);
19432016
return id;
19442017
}
19452018

1946-
return object_id0(obj);
2019+
return object_id0(obj, false);
19472020
}
19482021

19492022
static void
@@ -2045,8 +2118,18 @@ obj_free_object_id(VALUE obj)
20452118
break;
20462119
default: {
20472120
shape_id_t shape_id = RBASIC_SHAPE_ID(obj);
2048-
if (rb_shape_has_object_id(shape_id)) {
2121+
switch (rb_shape_has_object_id(shape_id)) {
2122+
case SHAPE_NO_OBJ_ID:
2123+
break;
2124+
case SHAPE_INLINE_OBJ_ID:
20492125
obj_id = object_id_get(obj, shape_id);
2126+
break;
2127+
case SHAPE_EXTERNAL_OBJ_ID:
2128+
// During shutdown the `obj_to_id_tbl` may be freed first.
2129+
if (obj_to_id_tbl) {
2130+
st_delete(obj_to_id_tbl, &obj, &obj_id);
2131+
}
2132+
break;
20502133
}
20512134
break;
20522135
}
@@ -2739,6 +2822,22 @@ rb_mark_set(st_table *tbl)
27392822
st_foreach(tbl, mark_key, (st_data_t)rb_gc_get_objspace());
27402823
}
27412824

2825+
static int
2826+
mark_value(st_data_t key, st_data_t value, st_data_t data)
2827+
{
2828+
gc_mark_internal((VALUE)value);
2829+
2830+
return ST_CONTINUE;
2831+
}
2832+
2833+
static void
2834+
mark_hash_values(st_table *tbl)
2835+
{
2836+
if (!tbl) return;
2837+
2838+
st_foreach(tbl, mark_value, 0);
2839+
}
2840+
27422841
static int
27432842
mark_keyvalue(st_data_t key, st_data_t value, st_data_t data)
27442843
{
@@ -4137,6 +4236,17 @@ rb_gc_vm_weak_table_foreach(vm_table_foreach_callback_func callback,
41374236
}
41384237
break;
41394238
}
4239+
case RB_GC_VM_OBJ_TO_ID_TABLE: {
4240+
if (obj_to_id_tbl) {
4241+
st_foreach_with_replace(
4242+
obj_to_id_tbl,
4243+
vm_weak_table_foreach_weak_key,
4244+
vm_weak_table_foreach_update_weak_key,
4245+
(st_data_t)&foreach_data
4246+
);
4247+
}
4248+
break;
4249+
}
41404250
case RB_GC_VM_GENERIC_FIELDS_TABLE: {
41414251
st_table *generic_fields_tbl = rb_generic_fields_tbl_get();
41424252
if (generic_fields_tbl) {
@@ -5543,6 +5653,7 @@ Init_GC(void)
55435653
{
55445654
#undef rb_intern
55455655
rb_gc_register_address(&id2ref_value);
5656+
rb_gc_register_address(&obj_to_id_value);
55465657

55475658
malloc_offset = gc_compute_malloc_offset();
55485659

‎gc/gc.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ enum rb_gc_vm_weak_tables {
2929
RB_GC_VM_OVERLOADED_CME_TABLE,
3030
RB_GC_VM_GLOBAL_SYMBOLS_TABLE,
3131
RB_GC_VM_ID2REF_TABLE,
32+
RB_GC_VM_OBJ_TO_ID_TABLE,
3233
RB_GC_VM_GENERIC_FIELDS_TABLE,
3334
RB_GC_VM_FROZEN_STRINGS_TABLE,
3435
RB_GC_VM_CC_REFINEMENT_TABLE,

‎shape.c‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,7 @@ remove_shape_recursive(rb_shape_t *shape, ID id, rb_shape_t **removed_shape)
709709
static inline shape_id_t transition_complex(shape_id_t shape_id);
710710

711711
static shape_id_t
712-
shape_transition_object_id(shape_id_t original_shape_id)
712+
shape_transition_object_id_inline(shape_id_t original_shape_id)
713713
{
714714
RUBY_ASSERT(!rb_shape_has_object_id(original_shape_id));
715715

@@ -720,17 +720,25 @@ shape_transition_object_id(shape_id_t original_shape_id)
720720
}
721721

722722
RUBY_ASSERT(shape);
723-
return shape_id(shape, original_shape_id) | SHAPE_ID_FL_HAS_OBJECT_ID;
723+
return shape_id(shape, original_shape_id) | SHAPE_ID_FL_OBJ_ID_INLINE;
724724
}
725725

726726
shape_id_t
727-
rb_shape_transition_object_id(VALUE obj)
727+
rb_shape_transition_object_id_inline(VALUE obj)
728728
{
729-
return shape_transition_object_id(RBASIC_SHAPE_ID(obj));
729+
return shape_transition_object_id_inline(RBASIC_SHAPE_ID(obj));
730730
}
731731

732732
shape_id_t
733-
rb_shape_object_id(shape_id_t original_shape_id)
733+
rb_shape_transition_object_id_external(VALUE obj)
734+
{
735+
shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj);
736+
RUBY_ASSERT(!rb_shape_has_object_id(original_shape_id));
737+
return original_shape_id | SHAPE_ID_FL_OBJ_ID_EXTERNAL;
738+
}
739+
740+
shape_id_t
741+
rb_shape_object_id_inline(shape_id_t original_shape_id)
734742
{
735743
RUBY_ASSERT(rb_shape_has_object_id(original_shape_id));
736744

@@ -742,7 +750,7 @@ rb_shape_object_id(shape_id_t original_shape_id)
742750
shape = RSHAPE(shape->parent_id);
743751
}
744752

745-
return shape_id(shape, original_shape_id) | SHAPE_ID_FL_HAS_OBJECT_ID;
753+
return shape_id(shape, original_shape_id) | SHAPE_ID_FL_OBJ_ID_INLINE;
746754
}
747755

748756
static inline shape_id_t
@@ -754,7 +762,7 @@ transition_complex(shape_id_t shape_id)
754762
if (heap_index) {
755763
next_shape_id = rb_shape_root(heap_index - 1) | SHAPE_ID_FL_TOO_COMPLEX;
756764
if (rb_shape_has_object_id(shape_id)) {
757-
next_shape_id = shape_transition_object_id(next_shape_id);
765+
next_shape_id = shape_transition_object_id_inline(next_shape_id);
758766
}
759767
}
760768
else {
@@ -1104,7 +1112,7 @@ rb_shape_rebuild(shape_id_t initial_shape_id, shape_id_t dest_shape_id)
11041112
return shape_id(next_shape, initial_shape_id);
11051113
}
11061114
else {
1107-
return transition_complex(initial_shape_id | (dest_shape_id & SHAPE_ID_FL_HAS_OBJECT_ID));
1115+
return transition_complex(initial_shape_id | (dest_shape_id & SHAPE_ID_FL_OBJ_ID_INLINE));
11081116
}
11091117
}
11101118

@@ -1223,7 +1231,7 @@ rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id)
12231231
shape = RSHAPE(shape->parent_id);
12241232
}
12251233

1226-
if (rb_shape_has_object_id(shape_id)) {
1234+
if (rb_shape_has_object_id(shape_id) == SHAPE_INLINE_OBJ_ID) {
12271235
if (!has_object_id) {
12281236
rb_p(obj);
12291237
rb_bug("shape_id claim having obj_id but doesn't shape_id=%u, obj=%s", shape_id, rb_obj_info(obj));

‎shape.h‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ STATIC_ASSERT(shape_id_num_bits, SHAPE_ID_NUM_BITS == sizeof(shape_id_t) * CHAR_
1414
#define SHAPE_ID_OFFSET_MASK (SHAPE_BUFFER_SIZE - 1)
1515
#define SHAPE_ID_FLAGS_MASK (shape_id_t)(((1 << (SHAPE_ID_NUM_BITS - SHAPE_ID_OFFSET_NUM_BITS)) - 1) << SHAPE_ID_OFFSET_NUM_BITS)
1616
#define SHAPE_ID_FL_FROZEN (SHAPE_FL_FROZEN << SHAPE_ID_OFFSET_NUM_BITS)
17-
#define SHAPE_ID_FL_HAS_OBJECT_ID (SHAPE_FL_HAS_OBJECT_ID << SHAPE_ID_OFFSET_NUM_BITS)
17+
#define SHAPE_ID_FL_OBJ_ID_INLINE (SHAPE_FL_OBJ_ID_INLINE << SHAPE_ID_OFFSET_NUM_BITS)
18+
#define SHAPE_ID_FL_OBJ_ID_EXTERNAL (SHAPE_FL_OBJ_ID_EXTERNAL << SHAPE_ID_OFFSET_NUM_BITS)
1819
#define SHAPE_ID_FL_TOO_COMPLEX (SHAPE_FL_TOO_COMPLEX << SHAPE_ID_OFFSET_NUM_BITS)
1920
#define SHAPE_ID_FL_NON_CANONICAL_MASK (SHAPE_FL_NON_CANONICAL_MASK << SHAPE_ID_OFFSET_NUM_BITS)
2021

@@ -42,7 +43,7 @@ typedef uint32_t redblack_id_t;
4243
#define ROOT_SHAPE_ID 0x0
4344
#define ROOT_SHAPE_WITH_OBJ_ID 0x1
4445
#define ROOT_TOO_COMPLEX_SHAPE_ID (ROOT_SHAPE_ID | SHAPE_ID_FL_TOO_COMPLEX)
45-
#define ROOT_TOO_COMPLEX_WITH_OBJ_ID (ROOT_SHAPE_WITH_OBJ_ID | SHAPE_ID_FL_TOO_COMPLEX | SHAPE_ID_FL_HAS_OBJECT_ID)
46+
#define ROOT_TOO_COMPLEX_WITH_OBJ_ID (ROOT_SHAPE_WITH_OBJ_ID | SHAPE_ID_FL_TOO_COMPLEX | SHAPE_ID_FL_OBJ_ID_INLINE)
4647
#define SPECIAL_CONST_SHAPE_ID (ROOT_SHAPE_ID | SHAPE_ID_FL_FROZEN)
4748

4849
extern ID ruby_internal_object_id;
@@ -76,10 +77,11 @@ enum shape_type {
7677

7778
enum shape_flags {
7879
SHAPE_FL_FROZEN = 1 << 0,
79-
SHAPE_FL_HAS_OBJECT_ID = 1 << 1,
80-
SHAPE_FL_TOO_COMPLEX = 1 << 2,
80+
SHAPE_FL_TOO_COMPLEX = 1 << 1,
81+
SHAPE_FL_OBJ_ID_INLINE = 1 << 2,
82+
SHAPE_FL_OBJ_ID_EXTERNAL = 1 << 3,
8183

82-
SHAPE_FL_NON_CANONICAL_MASK = SHAPE_FL_FROZEN | SHAPE_FL_HAS_OBJECT_ID,
84+
SHAPE_FL_NON_CANONICAL_MASK = SHAPE_FL_FROZEN | SHAPE_FL_OBJ_ID_INLINE | SHAPE_FL_OBJ_ID_EXTERNAL,
8385
};
8486

8587
typedef struct {
@@ -169,9 +171,10 @@ shape_id_t rb_shape_transition_complex(VALUE obj);
169171
shape_id_t rb_shape_transition_remove_ivar(VALUE obj, ID id, shape_id_t *removed_shape_id);
170172
shape_id_t rb_shape_transition_add_ivar(VALUE obj, ID id);
171173
shape_id_t rb_shape_transition_add_ivar_no_warnings(VALUE obj, ID id);
172-
shape_id_t rb_shape_transition_object_id(VALUE obj);
174+
shape_id_t rb_shape_transition_object_id_inline(VALUE obj);
175+
shape_id_t rb_shape_transition_object_id_external(VALUE obj);
173176
shape_id_t rb_shape_transition_heap(VALUE obj, size_t heap_index);
174-
shape_id_t rb_shape_object_id(shape_id_t original_shape_id);
177+
shape_id_t rb_shape_object_id_inline(shape_id_t original_shape_id);
175178

176179
void rb_shape_free_all(void);
177180

@@ -191,10 +194,22 @@ rb_shape_obj_too_complex_p(VALUE obj)
191194
return !RB_SPECIAL_CONST_P(obj) && rb_shape_too_complex_p(RBASIC_SHAPE_ID(obj));
192195
}
193196

194-
static inline bool
197+
enum rb_shape_object_id_type {
198+
SHAPE_NO_OBJ_ID = 0,
199+
SHAPE_INLINE_OBJ_ID = 1,
200+
SHAPE_EXTERNAL_OBJ_ID = 2,
201+
};
202+
203+
static inline enum rb_shape_object_id_type
195204
rb_shape_has_object_id(shape_id_t shape_id)
196205
{
197-
return shape_id & SHAPE_ID_FL_HAS_OBJECT_ID;
206+
if (shape_id & SHAPE_ID_FL_OBJ_ID_INLINE) {
207+
return SHAPE_INLINE_OBJ_ID;
208+
}
209+
else if (shape_id & SHAPE_ID_FL_OBJ_ID_EXTERNAL) {
210+
return SHAPE_EXTERNAL_OBJ_ID;
211+
}
212+
return SHAPE_NO_OBJ_ID;
198213
}
199214

200215
static inline bool
@@ -323,7 +338,7 @@ RBASIC_FIELDS_COUNT(VALUE obj)
323338

324339
bool rb_obj_set_shape_id(VALUE obj, shape_id_t shape_id);
325340

326-
static inline bool
341+
static inline enum rb_shape_object_id_type
327342
rb_shape_obj_has_id(VALUE obj)
328343
{
329344
return rb_shape_has_object_id(RBASIC_SHAPE_ID(obj));

0 commit comments

Comments
 (0)