diff --git a/st.c b/st.c index f11e9efaf9..70da7daf83 100644 --- a/st.c +++ b/st.c @@ -1495,7 +1495,16 @@ st_update(st_table *tab, st_data_t key, value = entry->record; } old_key = key; + + unsigned int rebuilds_num = tab->rebuilds_num; + retval = (*func)(&key, &value, arg, existing); + + // We need to make sure that the callback didn't cause a table rebuild + // Ideally we would make sure no operations happened + assert(rebuilds_num == tab->rebuilds_num); + (void)rebuilds_num; + switch (retval) { case ST_CONTINUE: if (! existing) { diff --git a/test/ruby/test_variable.rb b/test/ruby/test_variable.rb index 49fec2d40e..d8d0a1a393 100644 --- a/test/ruby/test_variable.rb +++ b/test/ruby/test_variable.rb @@ -407,6 +407,19 @@ class TestVariable < Test::Unit::TestCase } end + def test_exivar_resize_with_compaction_stress + objs = 10_000.times.map do + ExIvar.new + end + EnvUtil.under_gc_compact_stress do + 10.times do + x = ExIvar.new + x.instance_variable_set(:@resize, 1) + x + end + end + end + def test_local_variables_with_kwarg bug11674 = '[ruby-core:71437] [Bug #11674]' v = with_kwargs_11(v1:1,v2:2,v3:3,v4:4,v5:5,v6:6,v7:7,v8:8,v9:9,v10:10,v11:11) diff --git a/variable.c b/variable.c index 67dc2d3397..b18ec1018a 100644 --- a/variable.c +++ b/variable.c @@ -1823,34 +1823,6 @@ struct gen_fields_lookup_ensure_size { bool resize; }; -static int -generic_fields_lookup_ensure_size(st_data_t *k, st_data_t *v, st_data_t u, int existing) -{ - ASSERT_vm_locking(); - - struct gen_fields_lookup_ensure_size *fields_lookup = (struct gen_fields_lookup_ensure_size *)u; - struct gen_fields_tbl *fields_tbl = existing ? (struct gen_fields_tbl *)*v : NULL; - - if (!existing || fields_lookup->resize) { - if (existing) { - RUBY_ASSERT(RSHAPE_TYPE_P(fields_lookup->shape_id, SHAPE_IVAR) || RSHAPE_TYPE_P(fields_lookup->shape_id, SHAPE_OBJ_ID)); - RUBY_ASSERT(RSHAPE_CAPACITY(RSHAPE_PARENT(fields_lookup->shape_id)) < RSHAPE_CAPACITY(fields_lookup->shape_id)); - } - - fields_tbl = gen_fields_tbl_resize(fields_tbl, RSHAPE_CAPACITY(fields_lookup->shape_id)); - *v = (st_data_t)fields_tbl; - } - - fields_lookup->fields_tbl = fields_tbl; - if (fields_lookup->shape_id) { - rb_obj_set_shape_id(fields_lookup->obj, fields_lookup->shape_id); - } - - RUBY_ASSERT(rb_obj_exivar_p((VALUE)*k)); - - return ST_CONTINUE; -} - static VALUE * generic_ivar_set_shape_fields(VALUE obj, void *data) { @@ -1858,8 +1830,27 @@ generic_ivar_set_shape_fields(VALUE obj, void *data) struct gen_fields_lookup_ensure_size *fields_lookup = data; + // We can't use st_update, since when resizing the fields table GC can + // happen, which will modify the st_table and may rebuild it RB_VM_LOCKING() { - st_update(generic_fields_tbl(obj, fields_lookup->id, false), (st_data_t)obj, generic_fields_lookup_ensure_size, (st_data_t)fields_lookup); + struct gen_fields_tbl *fields_tbl = NULL; + st_table *tbl = generic_fields_tbl(obj, fields_lookup->id, false); + int existing = st_lookup(tbl, (st_data_t)obj, (st_data_t *)&fields_tbl); + + if (!existing || fields_lookup->resize) { + if (existing) { + RUBY_ASSERT(RSHAPE_TYPE_P(fields_lookup->shape_id, SHAPE_IVAR) || RSHAPE_TYPE_P(fields_lookup->shape_id, SHAPE_OBJ_ID)); + RUBY_ASSERT(RSHAPE_CAPACITY(RSHAPE_PARENT(fields_lookup->shape_id)) < RSHAPE_CAPACITY(fields_lookup->shape_id)); + } + + fields_tbl = gen_fields_tbl_resize(fields_tbl, RSHAPE_CAPACITY(fields_lookup->shape_id)); + st_insert(tbl, (st_data_t)obj, (st_data_t)fields_tbl); + } + + fields_lookup->fields_tbl = fields_tbl; + if (fields_lookup->shape_id) { + rb_obj_set_shape_id(fields_lookup->obj, fields_lookup->shape_id); + } } return fields_lookup->fields_tbl->as.shape.fields;