From 9232d8c8eb0b1cd0130d07c586a5adf14ca7d292 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 16 Nov 2023 18:27:59 +0100 Subject: [PATCH] Pin object ivars while they are being copied in a hash When transitioning an object to TOO_COMPLEX we copy all its ivar in a table, but if GC (and compaction) trigger in the middle of the transition, the old `iv_ptr` might still be the canonical ivar list and will be updated by the GC, but the reference we copied in the table will be outdated. So we must pin these reference until they are all copied and the object `iv_ptr` is pointing to the table. To do this I use a custom TypedData that simply mark the new table with `rb_mark_tbl` so references are pinned. --- internal/variable.h | 3 ++- object.c | 3 ++- variable.c | 34 ++++++++++++++++++++++++++++++++-- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/internal/variable.h b/internal/variable.h index 63b074a30884d6..14a5d4bbb41e48 100644 --- a/internal/variable.h +++ b/internal/variable.h @@ -47,7 +47,8 @@ VALUE rb_mod_set_temporary_name(VALUE, VALUE); struct gen_ivtbl; int rb_gen_ivtbl_get(VALUE obj, ID id, struct gen_ivtbl **ivtbl); -void rb_obj_copy_ivs_to_hash_table(VALUE obj, st_table *table); +VALUE rb_obj_copy_ivs_to_hash_table(VALUE obj, st_table *table); +void rb_obj_copy_ivs_to_hash_table_complete(VALUE ivar_pinner); void rb_obj_convert_to_too_complex(VALUE obj, st_table *table); void rb_evict_ivars_to_hash(VALUE obj); diff --git a/object.c b/object.c index a8090d0763db06..709e8ae1df9052 100644 --- a/object.c +++ b/object.c @@ -326,8 +326,9 @@ rb_obj_copy_ivar(VALUE dest, VALUE obj) shape_to_set_on_dest = rb_shape_rebuild_shape(initial_shape, src_shape); if (UNLIKELY(rb_shape_id(shape_to_set_on_dest) == OBJ_TOO_COMPLEX_SHAPE_ID)) { st_table * table = rb_st_init_numtable_with_size(src_num_ivs); - rb_obj_copy_ivs_to_hash_table(obj, table); + VALUE ivar_pinner = rb_obj_copy_ivs_to_hash_table(obj, table); rb_obj_convert_to_too_complex(dest, table); + rb_obj_copy_ivs_to_hash_table_complete(ivar_pinner); return; } diff --git a/variable.c b/variable.c index 793516209869e2..00add2c49cca92 100644 --- a/variable.c +++ b/variable.c @@ -1422,8 +1422,9 @@ rb_evict_ivars_to_hash(VALUE obj) st_table *table = st_init_numtable_with_size(rb_ivar_count(obj)); // Evacuate all previous values from shape into id_table - rb_obj_copy_ivs_to_hash_table(obj, table); + VALUE ivar_pinner = rb_obj_copy_ivs_to_hash_table(obj, table); rb_obj_convert_to_too_complex(obj, table); + rb_obj_copy_ivs_to_hash_table_complete(ivar_pinner); RUBY_ASSERT(rb_shape_obj_too_complex(obj)); } @@ -1647,10 +1648,39 @@ rb_obj_copy_ivs_to_hash_table_i(ID key, VALUE val, st_data_t arg) return ST_CONTINUE; } -void +static void +ivar_pinner_mark(void *data) { + st_table *table = (st_table *)data; + if (!data) { + return; + } + // rb_mark_tbl pin references + rb_mark_tbl(table); +} + +static const rb_data_type_t ivars_pinner_data_type = { + .wrap_struct_name = "ivar_pinner", + .function = { + .dmark = ivar_pinner_mark, + .dfree = NULL, + .dsize = NULL, + .dcompact = NULL, + }, + .flags = RUBY_TYPED_FREE_IMMEDIATELY, +}; + +VALUE rb_obj_copy_ivs_to_hash_table(VALUE obj, st_table *table) { + VALUE ivar_pinner = TypedData_Wrap_Struct(0, &ivars_pinner_data_type, table); rb_ivar_foreach(obj, rb_obj_copy_ivs_to_hash_table_i, (st_data_t)table); + return ivar_pinner; +} + +void +rb_obj_copy_ivs_to_hash_table_complete(VALUE ivar_pinner) +{ + RTYPEDDATA_DATA(ivar_pinner) = NULL; } static VALUE *