Fix crash in WeakMap during compaction

WeakMap can crash during compaction because the st_insert could allocate
memory.
This commit is contained in:
Peter Zhu 2023-09-06 14:20:23 -04:00
parent 746eede412
commit 12102d101a
4 changed files with 32 additions and 14 deletions

17
gc.c
View File

@ -9799,15 +9799,6 @@ gc_is_moveable_obj(rb_objspace_t *objspace, VALUE obj)
return FALSE; return FALSE;
} }
/* Used in places that could malloc, which can cause the GC to run. We need to
* temporarily disable the GC to allow the malloc to happen. */
#define COULD_MALLOC_REGION_START() \
GC_ASSERT(during_gc); \
VALUE _already_disabled = rb_gc_disable_no_rest(); \
#define COULD_MALLOC_REGION_END() \
if (_already_disabled == Qfalse) rb_objspace_gc_enable(objspace);
static VALUE static VALUE
gc_move(rb_objspace_t *objspace, VALUE scan, VALUE free, size_t src_slot_size, size_t slot_size) gc_move(rb_objspace_t *objspace, VALUE scan, VALUE free, size_t src_slot_size, size_t slot_size)
{ {
@ -9840,11 +9831,11 @@ gc_move(rb_objspace_t *objspace, VALUE scan, VALUE free, size_t src_slot_size, s
if (FL_TEST((VALUE)src, FL_EXIVAR)) { if (FL_TEST((VALUE)src, FL_EXIVAR)) {
/* Resizing the st table could cause a malloc */ /* Resizing the st table could cause a malloc */
COULD_MALLOC_REGION_START(); DURING_GC_COULD_MALLOC_REGION_START();
{ {
rb_mv_generic_ivar((VALUE)src, (VALUE)dest); rb_mv_generic_ivar((VALUE)src, (VALUE)dest);
} }
COULD_MALLOC_REGION_END(); DURING_GC_COULD_MALLOC_REGION_END();
} }
st_data_t srcid = (st_data_t)src, id; st_data_t srcid = (st_data_t)src, id;
@ -9854,12 +9845,12 @@ gc_move(rb_objspace_t *objspace, VALUE scan, VALUE free, size_t src_slot_size, s
if (st_lookup(objspace->obj_to_id_tbl, srcid, &id)) { if (st_lookup(objspace->obj_to_id_tbl, srcid, &id)) {
gc_report(4, objspace, "Moving object with seen id: %p -> %p\n", (void *)src, (void *)dest); gc_report(4, objspace, "Moving object with seen id: %p -> %p\n", (void *)src, (void *)dest);
/* Resizing the st table could cause a malloc */ /* Resizing the st table could cause a malloc */
COULD_MALLOC_REGION_START(); DURING_GC_COULD_MALLOC_REGION_START();
{ {
st_delete(objspace->obj_to_id_tbl, &srcid, 0); st_delete(objspace->obj_to_id_tbl, &srcid, 0);
st_insert(objspace->obj_to_id_tbl, (st_data_t)dest, id); st_insert(objspace->obj_to_id_tbl, (st_data_t)dest, id);
} }
COULD_MALLOC_REGION_END(); DURING_GC_COULD_MALLOC_REGION_END();
} }
/* Move the object */ /* Move the object */

View File

@ -189,6 +189,17 @@ struct rb_objspace; /* in vm_core.h */
# define SIZE_POOL_COUNT 5 # define SIZE_POOL_COUNT 5
#endif #endif
/* Used in places that could malloc during, which can cause the GC to run. We
* need to temporarily disable the GC to allow the malloc to happen.
* Allocating memory during GC is a bad idea, so use this only when absolutely
* necessary. */
#define DURING_GC_COULD_MALLOC_REGION_START() \
assert(rb_during_gc()); \
VALUE _already_disabled = rb_gc_disable_no_rest()
#define DURING_GC_COULD_MALLOC_REGION_END() \
if (_already_disabled == Qfalse) rb_gc_enable()
typedef struct ractor_newobj_size_pool_cache { typedef struct ractor_newobj_size_pool_cache {
struct RVALUE *freelist; struct RVALUE *freelist;
struct heap_page *using_page; struct heap_page *using_page;

View File

@ -223,6 +223,18 @@ class TestWeakMap < Test::Unit::TestCase
assert_equal(val, wm[key]) assert_equal(val, wm[key])
end; end;
assert_separately(["-W0"], <<-'end;')
wm = ObjectSpace::WeakMap.new
ary = 10_000.times.map do
o = Object.new
wm[o] = 1
o
end
GC.verify_compaction_references(expand_heap: true, toward: :empty)
end;
end end
def test_replaced_values_bug_19531 def test_replaced_values_bug_19531

View File

@ -122,7 +122,11 @@ wmap_compact_table_i(st_data_t key, st_data_t val, st_data_t data)
if (key_obj != new_key_obj) { if (key_obj != new_key_obj) {
*(VALUE *)key = new_key_obj; *(VALUE *)key = new_key_obj;
DURING_GC_COULD_MALLOC_REGION_START();
{
st_insert(table, key, val); st_insert(table, key, val);
}
DURING_GC_COULD_MALLOC_REGION_END();
return ST_DELETE; return ST_DELETE;
} }