diff --git a/cont.c b/cont.c index b33c1462bf3ea1..dbac05b05a0d5c 100644 --- a/cont.c +++ b/cont.c @@ -48,7 +48,8 @@ static const int DEBUG = 0; #define RB_PAGE_MASK (~(RB_PAGE_SIZE - 1)) static long pagesize; -static const rb_data_type_t cont_data_type, fiber_data_type; +const rb_data_type_t rb_cont_data_type; +const rb_data_type_t rb_fiber_data_type; static VALUE rb_cContinuation; static VALUE rb_cFiber; static VALUE rb_eFiberError; @@ -982,7 +983,7 @@ cont_ptr(VALUE obj) { rb_context_t *cont; - TypedData_Get_Struct(obj, rb_context_t, &cont_data_type, cont); + TypedData_Get_Struct(obj, rb_context_t, &rb_cont_data_type, cont); return cont; } @@ -992,7 +993,7 @@ fiber_ptr(VALUE obj) { rb_fiber_t *fiber; - TypedData_Get_Struct(obj, rb_fiber_t, &fiber_data_type, fiber); + TypedData_Get_Struct(obj, rb_fiber_t, &rb_fiber_data_type, fiber); if (!fiber) rb_raise(rb_eFiberError, "uninitialized fiber"); return fiber; @@ -1211,7 +1212,7 @@ fiber_memsize(const void *ptr) VALUE rb_obj_is_fiber(VALUE obj) { - return RBOOL(rb_typeddata_is_kind_of(obj, &fiber_data_type)); + return RBOOL(rb_typeddata_is_kind_of(obj, &rb_fiber_data_type)); } static void @@ -1241,13 +1242,27 @@ cont_save_machine_stack(rb_thread_t *th, rb_context_t *cont) asan_unpoison_memory_region(cont->machine.stack_src, size, false); MEMCPY(cont->machine.stack, cont->machine.stack_src, VALUE, size); } - -static const rb_data_type_t cont_data_type = { +const rb_data_type_t rb_cont_data_type = { "continuation", {cont_mark, cont_free, cont_memsize, cont_compact}, 0, 0, RUBY_TYPED_FREE_IMMEDIATELY }; +void +rb_cont_handle_weak_references(VALUE obj) +{ + rb_context_t *cont; + TypedData_Get_Struct(obj, rb_context_t, &rb_cont_data_type, cont); + + if (!cont) return; + + if (!rb_gc_handle_weak_references_alive_p(cont->saved_ec.gen_fields_cache.obj) || + !rb_gc_handle_weak_references_alive_p(cont->saved_ec.gen_fields_cache.fields_obj)) { + cont->saved_ec.gen_fields_cache.obj = Qundef; + cont->saved_ec.gen_fields_cache.fields_obj = Qundef; + } +} + static inline void cont_save_thread(rb_context_t *cont, rb_thread_t *th) { @@ -1404,7 +1419,8 @@ cont_new(VALUE klass) rb_thread_t *th = GET_THREAD(); THREAD_MUST_BE_RUNNING(th); - contval = TypedData_Make_Struct(klass, rb_context_t, &cont_data_type, cont); + contval = TypedData_Make_Struct(klass, rb_context_t, &rb_cont_data_type, cont); + rb_gc_declare_weak_references(contval); cont->self = contval; cont_init(cont, th); return cont; @@ -1983,16 +1999,33 @@ rb_cont_call(int argc, VALUE *argv, VALUE contval) * */ -static const rb_data_type_t fiber_data_type = { +const rb_data_type_t rb_fiber_data_type = { "fiber", {fiber_mark, fiber_free, fiber_memsize, fiber_compact,}, 0, 0, RUBY_TYPED_FREE_IMMEDIATELY }; +void +rb_fiber_handle_weak_references(VALUE obj) +{ + rb_fiber_t *fiber; + TypedData_Get_Struct(obj, rb_fiber_t, &rb_fiber_data_type, fiber); + + if (!fiber) return; + + if (!rb_gc_handle_weak_references_alive_p(fiber->cont.saved_ec.gen_fields_cache.obj) || + !rb_gc_handle_weak_references_alive_p(fiber->cont.saved_ec.gen_fields_cache.fields_obj)) { + fiber->cont.saved_ec.gen_fields_cache.obj = Qundef; + fiber->cont.saved_ec.gen_fields_cache.fields_obj = Qundef; + } +} + static VALUE fiber_alloc(VALUE klass) { - return TypedData_Wrap_Struct(klass, &fiber_data_type, 0); + VALUE obj = TypedData_Wrap_Struct(klass, &rb_fiber_data_type, 0); + rb_gc_declare_weak_references(obj); + return obj; } static rb_serial_t diff --git a/darray.h b/darray.h index c9035b74b642cd..dc9d282be28e7f 100644 --- a/darray.h +++ b/darray.h @@ -72,6 +72,20 @@ (*(ptr_to_ary))->meta.size++; \ } while (0) +/* Removes the element at idx and replaces it with the last element. + * ptr_to_ary and idx is evaluated multiple times. + * Warning: not bounds checked. + * + * void rb_darray_swap_remove(rb_darray(T) *ptr_to_ary, size_t idx); + */ +#define rb_darray_swap_remove(ptr_to_ary, idx) do { \ + size_t _darray_size = rb_darray_size(*(ptr_to_ary)); \ + if ((idx) != _darray_size - 1) { \ + (*(ptr_to_ary))->data[idx] = (*(ptr_to_ary))->data[_darray_size - 1]; \ + } \ + (*(ptr_to_ary))->meta.size--; \ +} while (0) + // Iterate over items of the array in a for loop // #define rb_darray_foreach(ary, idx_name, elem_ptr_var) \ diff --git a/gc.c b/gc.c index 8e239011e6f295..87db89d04956e6 100644 --- a/gc.c +++ b/gc.c @@ -637,8 +637,9 @@ typedef struct gc_function_map { void (*mark_and_move)(void *objspace_ptr, VALUE *ptr); void (*mark_and_pin)(void *objspace_ptr, VALUE obj); void (*mark_maybe)(void *objspace_ptr, VALUE obj); - void (*mark_weak)(void *objspace_ptr, VALUE *ptr); - void (*remove_weak)(void *objspace_ptr, VALUE parent_obj, VALUE *ptr); + // Weak references + void (*declare_weak_references)(void *objspace_ptr, VALUE obj); + bool (*handle_weak_references_alive_p)(void *objspace_ptr, VALUE obj); // Compaction bool (*object_moved_p)(void *objspace_ptr, VALUE obj); VALUE (*location)(void *objspace_ptr, VALUE value); @@ -811,8 +812,9 @@ ruby_modular_gc_init(void) load_modular_gc_func(mark_and_move); load_modular_gc_func(mark_and_pin); load_modular_gc_func(mark_maybe); - load_modular_gc_func(mark_weak); - load_modular_gc_func(remove_weak); + // Weak references + load_modular_gc_func(declare_weak_references); + load_modular_gc_func(handle_weak_references_alive_p); // Compaction load_modular_gc_func(object_moved_p); load_modular_gc_func(location); @@ -891,8 +893,9 @@ ruby_modular_gc_init(void) # define rb_gc_impl_mark_and_move rb_gc_functions.mark_and_move # define rb_gc_impl_mark_and_pin rb_gc_functions.mark_and_pin # define rb_gc_impl_mark_maybe rb_gc_functions.mark_maybe -# define rb_gc_impl_mark_weak rb_gc_functions.mark_weak -# define rb_gc_impl_remove_weak rb_gc_functions.remove_weak +// Weak references +# define rb_gc_impl_declare_weak_references rb_gc_functions.declare_weak_references +# define rb_gc_impl_handle_weak_references_alive_p rb_gc_functions.handle_weak_references_alive_p // Compaction # define rb_gc_impl_object_moved_p rb_gc_functions.object_moved_p # define rb_gc_impl_location rb_gc_functions.location @@ -1165,6 +1168,75 @@ rb_objspace_data_type_name(VALUE obj) } } +void +rb_gc_declare_weak_references(VALUE obj) +{ + rb_gc_impl_declare_weak_references(rb_gc_get_objspace(), obj); +} + +bool +rb_gc_handle_weak_references_alive_p(VALUE obj) +{ + if (SPECIAL_CONST_P(obj)) return true; + + return rb_gc_impl_handle_weak_references_alive_p(rb_gc_get_objspace(), obj); +} + +extern const rb_data_type_t rb_weakmap_type; +void rb_wmap_handle_weak_references(VALUE obj); +extern const rb_data_type_t rb_weakkeymap_type; +void rb_wkmap_handle_weak_references(VALUE obj); + +extern const rb_data_type_t rb_fiber_data_type; +void rb_fiber_handle_weak_references(VALUE obj); + +extern const rb_data_type_t rb_cont_data_type; +void rb_cont_handle_weak_references(VALUE obj); + +void +rb_gc_handle_weak_references(VALUE obj) +{ + switch (BUILTIN_TYPE(obj)) { + case T_DATA: + if (RTYPEDDATA_P(obj)) { + const rb_data_type_t *type = RTYPEDDATA_TYPE(obj); + + if (type == &rb_fiber_data_type) { + rb_fiber_handle_weak_references(obj); + } + else if (type == &rb_cont_data_type) { + rb_cont_handle_weak_references(obj); + } + else if (type == &rb_weakmap_type) { + rb_wmap_handle_weak_references(obj); + } + else if (type == &rb_weakkeymap_type) { + rb_wkmap_handle_weak_references(obj); + } + else { + rb_bug("rb_gc_handle_weak_references: unknown TypedData %s", RTYPEDDATA_TYPE(obj)->wrap_struct_name); + } + } + else { + rb_bug("rb_gc_handle_weak_references: unknown T_DATA"); + } + break; + + case T_IMEMO: { + GC_ASSERT(imemo_type(obj) == imemo_callcache); + + struct rb_callcache *cc = (struct rb_callcache *)obj; + if (!rb_gc_handle_weak_references_alive_p(cc->klass)) { + vm_cc_invalidate(cc); + } + + break; + } + default: + rb_bug("rb_gc_handle_weak_references: type not supported\n"); + } +} + static void io_fptr_finalize(void *fptr) { @@ -2649,29 +2721,6 @@ rb_gc_mark_maybe(VALUE obj) gc_mark_maybe_internal(obj); } -void -rb_gc_mark_weak(VALUE *ptr) -{ - if (RB_SPECIAL_CONST_P(*ptr)) return; - - rb_vm_t *vm = GET_VM(); - void *objspace = vm->gc.objspace; - if (LIKELY(vm->gc.mark_func_data == NULL)) { - GC_ASSERT(rb_gc_impl_during_gc_p(objspace)); - - rb_gc_impl_mark_weak(objspace, ptr); - } - else { - GC_ASSERT(!rb_gc_impl_during_gc_p(objspace)); - } -} - -void -rb_gc_remove_weak(VALUE parent_obj, VALUE *ptr) -{ - rb_gc_impl_remove_weak(rb_gc_get_objspace(), parent_obj, ptr); -} - ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS(static void each_location(register const VALUE *x, register long n, void (*cb)(VALUE, void *), void *data)); static void each_location(register const VALUE *x, register long n, void (*cb)(VALUE, void *), void *data) diff --git a/gc/default/default.c b/gc/default/default.c index ef100d7dea27dc..0e92c35598f09f 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -589,7 +589,6 @@ typedef struct rb_objspace { /* Weak references */ size_t weak_references_count; - size_t retained_weak_references_count; } profile; VALUE gc_stress_mode; @@ -635,7 +634,7 @@ typedef struct rb_objspace { VALUE stress_to_class; #endif - rb_darray(VALUE *) weak_references; + rb_darray(VALUE) weak_references; rb_postponed_job_handle_t finalize_deferred_pjob; unsigned long live_ractor_cache_count; @@ -4414,6 +4413,10 @@ gc_grey(rb_objspace_t *objspace, VALUE obj) MARK_IN_BITMAP(GET_HEAP_MARKING_BITS(obj), obj); } + if (RB_FL_TEST_RAW(obj, RUBY_FL_WEAK_REFERENCE)) { + rb_darray_append_without_gc(&objspace->weak_references, obj); + } + push_mark_stack(&objspace->mark_stack, obj); } @@ -4532,53 +4535,6 @@ rb_gc_impl_mark_maybe(void *objspace_ptr, VALUE obj) } } -void -rb_gc_impl_mark_weak(void *objspace_ptr, VALUE *ptr) -{ - rb_objspace_t *objspace = objspace_ptr; - - VALUE obj = *ptr; - - gc_mark_check_t_none(objspace, obj); - - /* If we are in a minor GC and the other object is old, then obj should - * already be marked and cannot be reclaimed in this GC cycle so we don't - * need to add it to the weak references list. */ - if (!is_full_marking(objspace) && RVALUE_OLD_P(objspace, obj)) { - GC_ASSERT(RVALUE_MARKED(objspace, obj)); - GC_ASSERT(!objspace->flags.during_compacting); - - return; - } - - rgengc_check_relation(objspace, obj); - - rb_darray_append_without_gc(&objspace->weak_references, ptr); - - objspace->profile.weak_references_count++; -} - -void -rb_gc_impl_remove_weak(void *objspace_ptr, VALUE parent_obj, VALUE *ptr) -{ - rb_objspace_t *objspace = objspace_ptr; - - /* If we're not incremental marking, then the state of the objects can't - * change so we don't need to do anything. */ - if (!is_incremental_marking(objspace)) return; - /* If parent_obj has not been marked, then ptr has not yet been marked - * weak, so we don't need to do anything. */ - if (!RVALUE_MARKED(objspace, parent_obj)) return; - - VALUE **ptr_ptr; - rb_darray_foreach(objspace->weak_references, i, ptr_ptr) { - if (*ptr_ptr == ptr) { - *ptr_ptr = NULL; - break; - } - } -} - static int pin_value(st_data_t key, st_data_t value, st_data_t data) { @@ -5362,30 +5318,40 @@ gc_marks_wb_unprotected_objects(rb_objspace_t *objspace, rb_heap_t *heap) gc_mark_stacked_objects_all(objspace); } -static void -gc_update_weak_references(rb_objspace_t *objspace) +void +rb_gc_impl_declare_weak_references(void *objspace_ptr, VALUE obj) { - size_t retained_weak_references_count = 0; - VALUE **ptr_ptr; - rb_darray_foreach(objspace->weak_references, i, ptr_ptr) { - if (!*ptr_ptr) continue; + FL_SET_RAW(obj, RUBY_FL_WEAK_REFERENCE); +} - VALUE obj = **ptr_ptr; +bool +rb_gc_impl_handle_weak_references_alive_p(void *objspace_ptr, VALUE obj) +{ + rb_objspace_t *objspace = objspace_ptr; - if (RB_SPECIAL_CONST_P(obj)) continue; + return RVALUE_MARKED(objspace, obj); +} - if (!RVALUE_MARKED(objspace, obj)) { - **ptr_ptr = Qundef; - } - else { - retained_weak_references_count++; - } +static void +gc_update_weak_references(rb_objspace_t *objspace) +{ + VALUE *obj_ptr; + rb_darray_foreach(objspace->weak_references, i, obj_ptr) { + rb_gc_handle_weak_references(*obj_ptr); } - objspace->profile.retained_weak_references_count = retained_weak_references_count; + size_t capa = rb_darray_capa(objspace->weak_references); + size_t size = rb_darray_size(objspace->weak_references); + + objspace->profile.weak_references_count = size; rb_darray_clear(objspace->weak_references); - rb_darray_resize_capa_without_gc(&objspace->weak_references, retained_weak_references_count); + + /* If the darray has capacity for more than four times the amount used, we + * shrink it down to half of that capacity. */ + if (capa > size * 4) { + rb_darray_resize_capa_without_gc(&objspace->weak_references, size * 2); + } } static void @@ -5942,6 +5908,10 @@ rgengc_rememberset_mark_plane(rb_objspace_t *objspace, uintptr_t p, bits_t bitse GC_ASSERT(RVALUE_OLD_P(objspace, obj) || RVALUE_WB_UNPROTECTED(objspace, obj)); gc_mark_children(objspace, obj); + + if (RB_FL_TEST_RAW(obj, RUBY_FL_WEAK_REFERENCE)) { + rb_darray_append_without_gc(&objspace->weak_references, obj); + } } p += BASE_SLOT_SIZE; bitset >>= 1; @@ -6503,7 +6473,6 @@ gc_start(rb_objspace_t *objspace, unsigned int reason) objspace->profile.total_allocated_objects_at_gc_start = total_allocated_objects(objspace); objspace->profile.heap_used_at_gc_start = rb_darray_size(objspace->heap_pages.sorted); objspace->profile.weak_references_count = 0; - objspace->profile.retained_weak_references_count = 0; gc_prof_setup_new_record(objspace, reason); gc_reset_malloc_info(objspace, do_full_mark); @@ -7323,7 +7292,7 @@ gc_info_decode(rb_objspace_t *objspace, const VALUE hash_or_key, const unsigned #endif static VALUE sym_newobj, sym_malloc, sym_method, sym_capi; static VALUE sym_none, sym_marking, sym_sweeping; - static VALUE sym_weak_references_count, sym_retained_weak_references_count; + static VALUE sym_weak_references_count; VALUE hash = Qnil, key = Qnil; VALUE major_by, need_major_by; unsigned int flags = orig_flags ? orig_flags : objspace->profile.latest_gc_info; @@ -7365,7 +7334,6 @@ gc_info_decode(rb_objspace_t *objspace, const VALUE hash_or_key, const unsigned S(sweeping); S(weak_references_count); - S(retained_weak_references_count); #undef S } @@ -7418,7 +7386,6 @@ gc_info_decode(rb_objspace_t *objspace, const VALUE hash_or_key, const unsigned } SET(weak_references_count, LONG2FIX(objspace->profile.weak_references_count)); - SET(retained_weak_references_count, LONG2FIX(objspace->profile.retained_weak_references_count)); #undef SET if (!NIL_P(key)) { diff --git a/gc/gc.h b/gc/gc.h index f1cf8038e0fe51..69dacf488f7db1 100644 --- a/gc/gc.h +++ b/gc/gc.h @@ -99,6 +99,7 @@ MODULAR_GC_FN void rb_gc_before_updating_jit_code(void); MODULAR_GC_FN void rb_gc_after_updating_jit_code(void); MODULAR_GC_FN bool rb_gc_obj_shareable_p(VALUE); MODULAR_GC_FN void rb_gc_rp(VALUE); +MODULAR_GC_FN void rb_gc_handle_weak_references(VALUE obj); #if USE_MODULAR_GC MODULAR_GC_FN bool rb_gc_event_hook_required_p(rb_event_flag_t event); diff --git a/gc/gc_impl.h b/gc/gc_impl.h index 3250483639e775..bb97fa4c09a4be 100644 --- a/gc/gc_impl.h +++ b/gc/gc_impl.h @@ -82,8 +82,9 @@ GC_IMPL_FN void rb_gc_impl_mark(void *objspace_ptr, VALUE obj); GC_IMPL_FN void rb_gc_impl_mark_and_move(void *objspace_ptr, VALUE *ptr); GC_IMPL_FN void rb_gc_impl_mark_and_pin(void *objspace_ptr, VALUE obj); GC_IMPL_FN void rb_gc_impl_mark_maybe(void *objspace_ptr, VALUE obj); -GC_IMPL_FN void rb_gc_impl_mark_weak(void *objspace_ptr, VALUE *ptr); -GC_IMPL_FN void rb_gc_impl_remove_weak(void *objspace_ptr, VALUE parent_obj, VALUE *ptr); +// Weak references +GC_IMPL_FN void rb_gc_impl_declare_weak_references(void *objspace_ptr, VALUE obj); +GC_IMPL_FN bool rb_gc_impl_handle_weak_references_alive_p(void *objspace_ptr, VALUE obj); // Compaction GC_IMPL_FN bool rb_gc_impl_object_moved_p(void *objspace_ptr, VALUE obj); GC_IMPL_FN VALUE rb_gc_impl_location(void *objspace_ptr, VALUE value); diff --git a/gc/mmtk/mmtk.c b/gc/mmtk/mmtk.c index 99c0125d97818d..634abc068a44f5 100644 --- a/gc/mmtk/mmtk.c +++ b/gc/mmtk/mmtk.c @@ -258,6 +258,12 @@ rb_mmtk_call_gc_mark_children(MMTk_ObjectReference object) rb_gc_mark_children(rb_gc_get_objspace(), (VALUE)object); } +static void +rb_mmtk_handle_weak_references(MMTk_ObjectReference object) +{ + rb_gc_handle_weak_references((VALUE)object); +} + static void rb_mmtk_call_obj_free(MMTk_ObjectReference object) { @@ -376,6 +382,7 @@ MMTk_RubyUpcalls ruby_upcalls = { rb_mmtk_scan_objspace, rb_mmtk_scan_object_ruby_style, rb_mmtk_call_gc_mark_children, + rb_mmtk_handle_weak_references, rb_mmtk_call_obj_free, rb_mmtk_vm_live_bytes, rb_mmtk_update_global_tables, @@ -406,7 +413,7 @@ void * rb_gc_impl_objspace_alloc(void) { MMTk_Builder *builder = rb_mmtk_builder_init(); - mmtk_init_binding(builder, NULL, &ruby_upcalls, (MMTk_ObjectReference)Qundef); + mmtk_init_binding(builder, NULL, &ruby_upcalls); return calloc(1, sizeof(struct objspace)); } @@ -768,16 +775,18 @@ rb_gc_impl_mark_maybe(void *objspace_ptr, VALUE obj) } } +// Weak references + void -rb_gc_impl_mark_weak(void *objspace_ptr, VALUE *ptr) +rb_gc_impl_declare_weak_references(void *objspace_ptr, VALUE obj) { - mmtk_mark_weak((MMTk_ObjectReference *)ptr); + mmtk_declare_weak_references((MMTk_ObjectReference)obj); } -void -rb_gc_impl_remove_weak(void *objspace_ptr, VALUE parent_obj, VALUE *ptr) +bool +rb_gc_impl_handle_weak_references_alive_p(void *objspace_ptr, VALUE obj) { - mmtk_remove_weak((MMTk_ObjectReference *)ptr); + return mmtk_weak_references_alive_p((MMTk_ObjectReference)obj); } // Compaction diff --git a/gc/mmtk/mmtk.h b/gc/mmtk/mmtk.h index 45521e2671b5cb..77e82cf06e5614 100644 --- a/gc/mmtk/mmtk.h +++ b/gc/mmtk/mmtk.h @@ -68,6 +68,7 @@ typedef struct MMTk_RubyUpcalls { void (*scan_objspace)(void); void (*scan_object_ruby_style)(MMTk_ObjectReference object); void (*call_gc_mark_children)(MMTk_ObjectReference object); + void (*handle_weak_references)(MMTk_ObjectReference object); void (*call_obj_free)(MMTk_ObjectReference object); size_t (*vm_live_bytes)(void); void (*update_global_tables)(int tbl_idx); @@ -91,8 +92,7 @@ MMTk_Builder *mmtk_builder_default(void); void mmtk_init_binding(MMTk_Builder *builder, const struct MMTk_RubyBindingOptions *_binding_options, - const struct MMTk_RubyUpcalls *upcalls, - MMTk_ObjectReference weak_reference_dead_value); + const struct MMTk_RubyUpcalls *upcalls); void mmtk_initialize_collection(MMTk_VMThread tls); @@ -121,9 +121,9 @@ void mmtk_post_alloc(MMTk_Mutator *mutator, void mmtk_add_obj_free_candidate(MMTk_ObjectReference object); -void mmtk_mark_weak(MMTk_ObjectReference *ptr); +void mmtk_declare_weak_references(MMTk_ObjectReference object); -void mmtk_remove_weak(const MMTk_ObjectReference *ptr); +bool mmtk_weak_references_alive_p(MMTk_ObjectReference object); void mmtk_object_reference_write_post(MMTk_Mutator *mutator, MMTk_ObjectReference object); diff --git a/gc/mmtk/src/abi.rs b/gc/mmtk/src/abi.rs index 1bd19fbe7efd1d..fed4d82f4e31e6 100644 --- a/gc/mmtk/src/abi.rs +++ b/gc/mmtk/src/abi.rs @@ -308,6 +308,7 @@ pub struct RubyUpcalls { pub scan_objspace: extern "C" fn(), pub scan_object_ruby_style: extern "C" fn(object: ObjectReference), pub call_gc_mark_children: extern "C" fn(object: ObjectReference), + pub handle_weak_references: extern "C" fn(object: ObjectReference), pub call_obj_free: extern "C" fn(object: ObjectReference), pub vm_live_bytes: extern "C" fn() -> usize, pub update_global_tables: extern "C" fn(tbl_idx: c_int), diff --git a/gc/mmtk/src/api.rs b/gc/mmtk/src/api.rs index d735a81cac810d..92146f87913c78 100644 --- a/gc/mmtk/src/api.rs +++ b/gc/mmtk/src/api.rs @@ -183,7 +183,6 @@ pub unsafe extern "C" fn mmtk_init_binding( builder: *mut MMTKBuilder, _binding_options: *const RubyBindingOptions, upcalls: *const RubyUpcalls, - weak_reference_dead_value: ObjectReference, ) { crate::MUTATOR_THREAD_PANIC_HANDLER .set((unsafe { (*upcalls).clone() }).mutator_thread_panic_handler) @@ -203,7 +202,6 @@ pub unsafe extern "C" fn mmtk_init_binding( mmtk_static, &binding_options, upcalls, - weak_reference_dead_value, ); crate::BINDING @@ -306,16 +304,16 @@ pub extern "C" fn mmtk_add_obj_free_candidate(object: ObjectReference) { binding().weak_proc.add_obj_free_candidate(object) } -// =============== Marking =============== +// =============== Weak references =============== #[no_mangle] -pub extern "C" fn mmtk_mark_weak(ptr: &'static mut ObjectReference) { - binding().weak_proc.add_weak_reference(ptr); +pub extern "C" fn mmtk_declare_weak_references(object: ObjectReference) { + binding().weak_proc.add_weak_reference(object); } #[no_mangle] -pub extern "C" fn mmtk_remove_weak(ptr: &ObjectReference) { - binding().weak_proc.remove_weak_reference(ptr); +pub extern "C" fn mmtk_weak_references_alive_p(object: ObjectReference) -> bool { + object.is_reachable() } // =============== Write barriers =============== diff --git a/gc/mmtk/src/binding.rs b/gc/mmtk/src/binding.rs index 811cbf8abf8b32..ddbaafb0767b1f 100644 --- a/gc/mmtk/src/binding.rs +++ b/gc/mmtk/src/binding.rs @@ -56,8 +56,6 @@ pub struct RubyBinding { pub weak_proc: WeakProcessor, pub gc_thread_join_handles: Mutex>>, pub wb_unprotected_objects: Mutex>, - - pub weak_reference_dead_value: ObjectReference, } unsafe impl Sync for RubyBinding {} @@ -68,7 +66,6 @@ impl RubyBinding { mmtk: &'static MMTK, binding_options: &RubyBindingOptions, upcalls: *const abi::RubyUpcalls, - weak_reference_dead_value: ObjectReference, ) -> Self { unsafe { crate::BINDING_FAST.suffix_size = binding_options.suffix_size; @@ -82,8 +79,6 @@ impl RubyBinding { weak_proc: WeakProcessor::new(), gc_thread_join_handles: Default::default(), wb_unprotected_objects: Default::default(), - - weak_reference_dead_value, } } diff --git a/gc/mmtk/src/weak_proc.rs b/gc/mmtk/src/weak_proc.rs index 68d3dd3273a135..ce80d323495e73 100644 --- a/gc/mmtk/src/weak_proc.rs +++ b/gc/mmtk/src/weak_proc.rs @@ -13,7 +13,7 @@ pub struct WeakProcessor { /// If it is a bottleneck, replace it with a lock-free data structure, /// or add candidates in batch. obj_free_candidates: Mutex>, - weak_references: Mutex>, + weak_references: Mutex>, } impl Default for WeakProcessor { @@ -53,19 +53,9 @@ impl WeakProcessor { std::mem::take(obj_free_candidates.as_mut()) } - pub fn add_weak_reference(&self, ptr: &'static mut ObjectReference) { + pub fn add_weak_reference(&self, object: ObjectReference) { let mut weak_references = self.weak_references.lock().unwrap(); - weak_references.push(ptr); - } - - pub fn remove_weak_reference(&self, ptr: &ObjectReference) { - let mut weak_references = self.weak_references.lock().unwrap(); - for (i, curr_ptr) in weak_references.iter().enumerate() { - if *curr_ptr == ptr { - weak_references.swap_remove(i); - break; - } - } + weak_references.push(object); } pub fn process_weak_stuff( @@ -133,17 +123,15 @@ impl GCWork for ProcessWeakReferences { .try_lock() .expect("Mutators should not be holding the lock."); - for ptr_ptr in weak_references.iter_mut() { - if (upcalls().special_const_p)(**ptr_ptr) { - continue; - } + weak_references.retain(|&object| { + if object.is_reachable() { + (upcalls().handle_weak_references)(object); - if !(**ptr_ptr).is_reachable() { - **ptr_ptr = crate::binding().weak_reference_dead_value; + true + } else { + false } - } - - weak_references.clear(); + }); } } diff --git a/imemo.c b/imemo.c index 42f6615a5e83be..2d63b7cd99549a 100644 --- a/imemo.c +++ b/imemo.c @@ -372,7 +372,6 @@ rb_imemo_mark_and_move(VALUE obj, bool reference_updating) RUBY_ASSERT(RB_TYPE_P(cc->klass, T_CLASS) || RB_TYPE_P(cc->klass, T_ICLASS)); RUBY_ASSERT(IMEMO_TYPE_P((VALUE)cc->cme_, imemo_ment)); - rb_gc_mark_weak((VALUE *)&cc->klass); if ((vm_cc_super_p(cc) || vm_cc_refinement_p(cc))) { rb_gc_mark_movable((VALUE)cc->cme_); } diff --git a/include/ruby/internal/fl_type.h b/include/ruby/internal/fl_type.h index 9bbcb9d2b82433..456ec77b87490a 100644 --- a/include/ruby/internal/fl_type.h +++ b/include/ruby/internal/fl_type.h @@ -295,11 +295,11 @@ ruby_fl_type { = 0, /** - * This flag is no longer in use + * This object weakly refers to other objects. * * @internal */ - RUBY_FL_UNUSED9 = (1<<9), + RUBY_FL_WEAK_REFERENCE = (1<<9), /** * This flag is no longer in use diff --git a/internal/gc.h b/internal/gc.h index ea001449a0030c..8fff2e83361192 100644 --- a/internal/gc.h +++ b/internal/gc.h @@ -214,8 +214,8 @@ size_t rb_gc_heap_id_for_size(size_t size); void rb_gc_mark_and_move(VALUE *ptr); -void rb_gc_mark_weak(VALUE *ptr); -void rb_gc_remove_weak(VALUE parent_obj, VALUE *ptr); +void rb_gc_declare_weak_references(VALUE obj); +bool rb_gc_handle_weak_references_alive_p(VALUE obj); void rb_gc_ref_update_table_values_only(st_table *tbl); diff --git a/test/ruby/test_gc.rb b/test/ruby/test_gc.rb index 6639013a54ca32..594e2b8aa8a4ca 100644 --- a/test/ruby/test_gc.rb +++ b/test/ruby/test_gc.rb @@ -382,51 +382,36 @@ def test_latest_gc_info_need_major_by def test_latest_gc_info_weak_references_count assert_separately([], __FILE__, __LINE__, <<~RUBY) GC.disable - count = 10_000 + COUNT = 10_000 # Some weak references may be created, so allow some margin of error error_tolerance = 100 - # Run full GC to clear out weak references - GC.start - # Run full GC again to collect stats about weak references + # Run full GC to collect stats about weak references GC.start before_weak_references_count = GC.latest_gc_info(:weak_references_count) - before_retained_weak_references_count = GC.latest_gc_info(:retained_weak_references_count) - # Create some objects and place it in a WeakMap - wmap = ObjectSpace::WeakMap.new - ary = Array.new(count) do |i| - obj = Object.new - wmap[obj] = nil - obj + # Create some WeakMaps + ary = Array.new(COUNT) + COUNT.times.with_index do |i| + ary[i] = ObjectSpace::WeakMap.new end # Run full GC to collect stats about weak references GC.start - assert_operator(GC.latest_gc_info(:weak_references_count), :>=, before_weak_references_count + count - error_tolerance) - assert_operator(GC.latest_gc_info(:retained_weak_references_count), :>=, before_retained_weak_references_count + count - error_tolerance) - assert_operator(GC.latest_gc_info(:retained_weak_references_count), :<=, GC.latest_gc_info(:weak_references_count)) + assert_operator(GC.latest_gc_info(:weak_references_count), :>=, before_weak_references_count + COUNT - error_tolerance) before_weak_references_count = GC.latest_gc_info(:weak_references_count) - before_retained_weak_references_count = GC.latest_gc_info(:retained_weak_references_count) # Clear ary, so if ary itself is somewhere on the stack, it won't hold all references ary.clear ary = nil - # Free ary, which should empty out the wmap + # Free ary, which should GC all the WeakMaps GC.start - # Run full GC again to collect stats about weak references - GC.start - - # Sometimes the WeakMap has a few elements, which might be held on by registers. - assert_operator(wmap.size, :<=, count / 1000) - assert_operator(GC.latest_gc_info(:weak_references_count), :<=, before_weak_references_count - count + error_tolerance) - assert_operator(GC.latest_gc_info(:retained_weak_references_count), :<=, before_retained_weak_references_count - count + error_tolerance) - assert_operator(GC.latest_gc_info(:retained_weak_references_count), :<=, GC.latest_gc_info(:weak_references_count)) + assert_operator(GC.latest_gc_info(:weak_references_count), :<=, before_weak_references_count - COUNT + error_tolerance) RUBY end diff --git a/vm.c b/vm.c index f78a779c3f3655..0c3a2d01a515d4 100644 --- a/vm.c +++ b/vm.c @@ -3733,9 +3733,6 @@ rb_execution_context_mark(const rb_execution_context_t *ec) rb_gc_mark(ec->private_const_reference); rb_gc_mark_movable(ec->storage); - - rb_gc_mark_weak((VALUE *)&ec->gen_fields_cache.obj); - rb_gc_mark_weak((VALUE *)&ec->gen_fields_cache.fields_obj); } void rb_fiber_mark_self(rb_fiber_t *fib); diff --git a/vm_callinfo.h b/vm_callinfo.h index 2c51bf9092046e..3df4b4eb0e9500 100644 --- a/vm_callinfo.h +++ b/vm_callinfo.h @@ -342,6 +342,8 @@ vm_cc_new(VALUE klass, { cc_check_class(klass); struct rb_callcache *cc = SHAREABLE_IMEMO_NEW(struct rb_callcache, imemo_callcache, klass); + rb_gc_declare_weak_references((VALUE)cc); + *((struct rb_callable_method_entry_struct **)&cc->cme_) = (struct rb_callable_method_entry_struct *)cme; *((vm_call_handler *)&cc->call_) = call; diff --git a/weakmap.c b/weakmap.c index ecdd219f62a21e..7b6f27ce2bbbed 100644 --- a/weakmap.c +++ b/weakmap.c @@ -34,102 +34,11 @@ struct weakmap_entry { VALUE val; }; -static bool -wmap_live_p(VALUE obj) -{ - return !UNDEF_P(obj); -} - -struct wmap_foreach_data { - int (*func)(struct weakmap_entry *, st_data_t); - st_data_t arg; - - struct weakmap_entry *dead_entry; -}; - -static int -wmap_foreach_i(st_data_t key, st_data_t val, st_data_t arg) -{ - struct wmap_foreach_data *data = (struct wmap_foreach_data *)arg; - - if (data->dead_entry != NULL) { - ruby_sized_xfree(data->dead_entry, sizeof(struct weakmap_entry)); - data->dead_entry = NULL; - } - - struct weakmap_entry *entry = (struct weakmap_entry *)key; - RUBY_ASSERT(&entry->val == (VALUE *)val); - - if (wmap_live_p(entry->key) && wmap_live_p(entry->val)) { - VALUE k = entry->key; - VALUE v = entry->val; - - int ret = data->func(entry, data->arg); - - RB_GC_GUARD(k); - RB_GC_GUARD(v); - - return ret; - } - else { - /* We cannot free the weakmap_entry here because the ST_DELETE could - * hash the key which would read the weakmap_entry and would cause a - * use-after-free. Instead, we store this entry and free it on the next - * iteration. */ - data->dead_entry = entry; - - return ST_DELETE; - } -} - -static void -wmap_foreach(struct weakmap *w, int (*func)(struct weakmap_entry *, st_data_t), st_data_t arg) -{ - struct wmap_foreach_data foreach_data = { - .func = func, - .arg = arg, - .dead_entry = NULL, - }; - - st_foreach(w->table, wmap_foreach_i, (st_data_t)&foreach_data); - - ruby_sized_xfree(foreach_data.dead_entry, sizeof(struct weakmap_entry)); -} - -static int -wmap_mark_weak_table_i(struct weakmap_entry *entry, st_data_t _) -{ - rb_gc_mark_weak(&entry->key); - rb_gc_mark_weak(&entry->val); - - return ST_CONTINUE; -} - -static void -wmap_mark(void *ptr) -{ - struct weakmap *w = ptr; - if (w->table) { - wmap_foreach(w, wmap_mark_weak_table_i, (st_data_t)0); - } -} - -static int -wmap_free_table_i(st_data_t key, st_data_t val, st_data_t arg) -{ - struct weakmap_entry *entry = (struct weakmap_entry *)key; - RUBY_ASSERT(&entry->val == (VALUE *)val); - ruby_sized_xfree(entry, sizeof(struct weakmap_entry)); - - return ST_CONTINUE; -} - static void wmap_free(void *ptr) { struct weakmap *w = ptr; - st_foreach(w->table, wmap_free_table_i, 0); st_free_table(w->table); } @@ -154,40 +63,37 @@ struct wmap_compact_table_data { }; static int -wmap_compact_table_i(st_data_t key, st_data_t val, st_data_t d) +wmap_compact_table_each_i(st_data_t k, st_data_t v, st_data_t d, int error) { - struct wmap_compact_table_data *data = (struct wmap_compact_table_data *)d; - if (data->dead_entry != NULL) { - ruby_sized_xfree(data->dead_entry, sizeof(struct weakmap_entry)); - data->dead_entry = NULL; - } - - struct weakmap_entry *entry = (struct weakmap_entry *)key; + st_table *table = (st_table *)d; - entry->val = rb_gc_location(entry->val); + VALUE key = (VALUE)k; + VALUE val = (VALUE)v; - VALUE new_key = rb_gc_location(entry->key); + VALUE moved_key = rb_gc_location(key); + VALUE moved_val = rb_gc_location(val); /* If the key object moves, then we must reinsert because the hash is * based on the pointer rather than the object itself. */ - if (entry->key != new_key) { - DURING_GC_COULD_MALLOC_REGION_START(); - { - struct weakmap_entry *new_entry = xmalloc(sizeof(struct weakmap_entry)); - new_entry->key = new_key; - new_entry->val = entry->val; - st_insert(data->table, (st_data_t)&new_entry->key, (st_data_t)&new_entry->val); - } - DURING_GC_COULD_MALLOC_REGION_END(); - - /* We cannot free the weakmap_entry here because the ST_DELETE could - * hash the key which would read the weakmap_entry and would cause a - * use-after-free. Instead, we store this entry and free it on the next - * iteration. */ - data->dead_entry = entry; + if (key != moved_key) { + st_insert(table, (st_data_t)moved_key, (st_data_t)moved_val); return ST_DELETE; } + else if (val != moved_val) { + return ST_REPLACE; + } + else { + return ST_CONTINUE; + } +} + +static int +wmap_compact_table_replace_i(st_data_t *k, st_data_t *v, st_data_t d, int existing) +{ + RUBY_ASSERT((VALUE)*k == rb_gc_location((VALUE)*k)); + + *v = (st_data_t)rb_gc_location((VALUE)*v); return ST_CONTINUE; } @@ -198,21 +104,18 @@ wmap_compact(void *ptr) struct weakmap *w = ptr; if (w->table) { - struct wmap_compact_table_data compact_data = { - .table = w->table, - .dead_entry = NULL, - }; - - st_foreach(w->table, wmap_compact_table_i, (st_data_t)&compact_data); - - ruby_sized_xfree(compact_data.dead_entry, sizeof(struct weakmap_entry)); + DURING_GC_COULD_MALLOC_REGION_START(); + { + st_foreach_with_replace(w->table, wmap_compact_table_each_i, wmap_compact_table_replace_i, (st_data_t)w->table); + } + DURING_GC_COULD_MALLOC_REGION_END(); } } -static const rb_data_type_t weakmap_type = { +const rb_data_type_t rb_weakmap_type = { "weakmap", { - wmap_mark, + NULL, wmap_free, wmap_memsize, wmap_compact, @@ -223,21 +126,13 @@ static const rb_data_type_t weakmap_type = { static int wmap_cmp(st_data_t x, st_data_t y) { - VALUE x_obj = *(VALUE *)x; - VALUE y_obj = *(VALUE *)y; - - if (!wmap_live_p(x_obj) && !wmap_live_p(y_obj)) { - return x != y; - } - else { - return x_obj != y_obj; - } + return x != y; } static st_index_t wmap_hash(st_data_t n) { - return st_numhash(*(VALUE *)n); + return st_numhash(n); } static const struct st_hash_type wmap_hash_type = { @@ -245,12 +140,37 @@ static const struct st_hash_type wmap_hash_type = { wmap_hash, }; +static int +rb_wmap_handle_weak_references_i(st_data_t key, st_data_t val, st_data_t arg) +{ + if (rb_gc_handle_weak_references_alive_p(key) && + rb_gc_handle_weak_references_alive_p(val)) { + return ST_CONTINUE; + } + else { + return ST_DELETE; + } +} + +void +rb_wmap_handle_weak_references(VALUE self) +{ + struct weakmap *w; + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); + + st_foreach(w->table, rb_wmap_handle_weak_references_i, (st_data_t)0); +} + static VALUE wmap_allocate(VALUE klass) { struct weakmap *w; - VALUE obj = TypedData_Make_Struct(klass, struct weakmap, &weakmap_type, w); + VALUE obj = TypedData_Make_Struct(klass, struct weakmap, &rb_weakmap_type, w); + w->table = st_init_table(&wmap_hash_type); + + rb_gc_declare_weak_references(obj); + return obj; } @@ -266,8 +186,10 @@ wmap_inspect_append(VALUE str, VALUE obj) } static int -wmap_inspect_i(struct weakmap_entry *entry, st_data_t data) +wmap_inspect_i(st_data_t k, st_data_t v, st_data_t data) { + VALUE key = (VALUE)k; + VALUE val = (VALUE)v; VALUE str = (VALUE)data; if (RSTRING_PTR(str)[0] == '#') { @@ -278,9 +200,9 @@ wmap_inspect_i(struct weakmap_entry *entry, st_data_t data) RSTRING_PTR(str)[0] = '#'; } - wmap_inspect_append(str, entry->key); + wmap_inspect_append(str, key); rb_str_cat2(str, " => "); - wmap_inspect_append(str, entry->val); + wmap_inspect_append(str, val); return ST_CONTINUE; } @@ -290,11 +212,11 @@ wmap_inspect(VALUE self) { VALUE c = rb_class_name(CLASS_OF(self)); struct weakmap *w; - TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); VALUE str = rb_sprintf("-<%"PRIsVALUE":%p", c, (void *)self); - wmap_foreach(w, wmap_inspect_i, (st_data_t)str); + st_foreach(w->table, wmap_inspect_i, (st_data_t)str); RSTRING_PTR(str)[0] = '#'; rb_str_cat2(str, ">"); @@ -303,9 +225,9 @@ wmap_inspect(VALUE self) } static int -wmap_each_i(struct weakmap_entry *entry, st_data_t _) +wmap_each_i(st_data_t k, st_data_t v, st_data_t _) { - rb_yield_values(2, entry->key, entry->val); + rb_yield_values(2, (VALUE)k, (VALUE)v); return ST_CONTINUE; } @@ -322,17 +244,17 @@ static VALUE wmap_each(VALUE self) { struct weakmap *w; - TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); - wmap_foreach(w, wmap_each_i, (st_data_t)0); + st_foreach(w->table, wmap_each_i, (st_data_t)0); return self; } static int -wmap_each_key_i(struct weakmap_entry *entry, st_data_t _data) +wmap_each_key_i(st_data_t k, st_data_t _v, st_data_t _data) { - rb_yield(entry->key); + rb_yield((VALUE)k); return ST_CONTINUE; } @@ -349,17 +271,17 @@ static VALUE wmap_each_key(VALUE self) { struct weakmap *w; - TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); - wmap_foreach(w, wmap_each_key_i, (st_data_t)0); + st_foreach(w->table, wmap_each_key_i, (st_data_t)0); return self; } static int -wmap_each_value_i(struct weakmap_entry *entry, st_data_t _data) +wmap_each_value_i(st_data_t k, st_data_t v, st_data_t _data) { - rb_yield(entry->val); + rb_yield((VALUE)v); return ST_CONTINUE; } @@ -376,19 +298,19 @@ static VALUE wmap_each_value(VALUE self) { struct weakmap *w; - TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); - wmap_foreach(w, wmap_each_value_i, (st_data_t)0); + st_foreach(w->table, wmap_each_value_i, (st_data_t)0); return self; } static int -wmap_keys_i(struct weakmap_entry *entry, st_data_t arg) +wmap_keys_i(st_data_t k, st_data_t v, st_data_t data) { - VALUE ary = (VALUE)arg; + VALUE ary = (VALUE)data; - rb_ary_push(ary, entry->key); + rb_ary_push(ary, (VALUE)k); return ST_CONTINUE; } @@ -404,20 +326,20 @@ static VALUE wmap_keys(VALUE self) { struct weakmap *w; - TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); VALUE ary = rb_ary_new(); - wmap_foreach(w, wmap_keys_i, (st_data_t)ary); + st_foreach(w->table, wmap_keys_i, (st_data_t)ary); return ary; } static int -wmap_values_i(struct weakmap_entry *entry, st_data_t arg) +wmap_values_i(st_data_t k, st_data_t v, st_data_t data) { - VALUE ary = (VALUE)arg; + VALUE ary = (VALUE)data; - rb_ary_push(ary, entry->val); + rb_ary_push(ary, (VALUE)v); return ST_CONTINUE; } @@ -433,36 +355,14 @@ static VALUE wmap_values(VALUE self) { struct weakmap *w; - TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); VALUE ary = rb_ary_new(); - wmap_foreach(w, wmap_values_i, (st_data_t)ary); + st_foreach(w->table, wmap_values_i, (st_data_t)ary); return ary; } -static int -wmap_aset_replace(st_data_t *key, st_data_t *val, st_data_t new_key_ptr, int existing) -{ - VALUE new_key = *(VALUE *)new_key_ptr; - VALUE new_val = *(((VALUE *)new_key_ptr) + 1); - - if (existing) { - RUBY_ASSERT(*(VALUE *)*key == new_key); - } - else { - struct weakmap_entry *entry = xmalloc(sizeof(struct weakmap_entry)); - - *key = (st_data_t)&entry->key; - *val = (st_data_t)&entry->val; - } - - *(VALUE *)*key = new_key; - *(VALUE *)*val = new_val; - - return ST_CONTINUE; -} - /* * call-seq: * map[key] = value -> value @@ -476,11 +376,9 @@ static VALUE wmap_aset(VALUE self, VALUE key, VALUE val) { struct weakmap *w; - TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); - - VALUE pair[2] = { key, val }; + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); - st_update(w->table, (st_data_t)pair, wmap_aset_replace, (st_data_t)pair); + st_insert(w->table, (st_data_t)key, (st_data_t)val); RB_OBJ_WRITTEN(self, Qundef, key); RB_OBJ_WRITTEN(self, Qundef, val); @@ -492,17 +390,13 @@ wmap_aset(VALUE self, VALUE key, VALUE val) static VALUE wmap_lookup(VALUE self, VALUE key) { - RUBY_ASSERT(wmap_live_p(key)); - struct weakmap *w; - TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); st_data_t data; - if (!st_lookup(w->table, (st_data_t)&key, &data)) return Qundef; + if (!st_lookup(w->table, (st_data_t)key, &data)) return Qundef; - if (!wmap_live_p(*(VALUE *)data)) return Qundef; - - return *(VALUE *)data; + return (VALUE)data; } /* @@ -552,23 +446,12 @@ static VALUE wmap_delete(VALUE self, VALUE key) { struct weakmap *w; - TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); - - VALUE orig_key = key; - st_data_t orig_key_data = (st_data_t)&orig_key; - st_data_t orig_val_data; - if (st_delete(w->table, &orig_key_data, &orig_val_data)) { - VALUE orig_val = *(VALUE *)orig_val_data; - - rb_gc_remove_weak(self, (VALUE *)orig_key_data); - rb_gc_remove_weak(self, (VALUE *)orig_val_data); - - struct weakmap_entry *entry = (struct weakmap_entry *)orig_key_data; - ruby_sized_xfree(entry, sizeof(struct weakmap_entry)); + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); - if (wmap_live_p(orig_val)) { - return orig_val; - } + st_data_t orig_key = (st_data_t)key; + st_data_t orig_val; + if (st_delete(w->table, &orig_key, &orig_val)) { + return (VALUE)orig_val; } if (rb_block_given_p()) { @@ -601,7 +484,7 @@ static VALUE wmap_size(VALUE self) { struct weakmap *w; - TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + TypedData_Get_Struct(self, struct weakmap, &rb_weakmap_type, w); st_index_t n = st_table_size(w->table); @@ -635,25 +518,11 @@ struct weakkeymap { }; static int -wkmap_mark_table_i(st_data_t key, st_data_t val_obj, st_data_t data) +wkmap_mark_table_i(st_data_t key, st_data_t val_obj, st_data_t _data) { - VALUE **dead_entry = (VALUE **)data; - ruby_sized_xfree(*dead_entry, sizeof(VALUE)); - *dead_entry = NULL; - - VALUE *key_ptr = (VALUE *)key; - - if (wmap_live_p(*key_ptr)) { - rb_gc_mark_weak(key_ptr); - rb_gc_mark_movable((VALUE)val_obj); + rb_gc_mark_movable((VALUE)val_obj); - return ST_CONTINUE; - } - else { - *dead_entry = key_ptr; - - return ST_DELETE; - } + return ST_CONTINUE; } static void @@ -661,27 +530,15 @@ wkmap_mark(void *ptr) { struct weakkeymap *w = ptr; if (w->table) { - VALUE *dead_entry = NULL; - st_foreach(w->table, wkmap_mark_table_i, (st_data_t)&dead_entry); - if (dead_entry != NULL) { - ruby_sized_xfree(dead_entry, sizeof(VALUE)); - } + st_foreach(w->table, wkmap_mark_table_i, (st_data_t)0); } } -static int -wkmap_free_table_i(st_data_t key, st_data_t _val, st_data_t _arg) -{ - ruby_sized_xfree((VALUE *)key, sizeof(VALUE)); - return ST_CONTINUE; -} - static void wkmap_free(void *ptr) { struct weakkeymap *w = ptr; - st_foreach(w->table, wkmap_free_table_i, 0); st_free_table(w->table); } @@ -701,26 +558,13 @@ wkmap_memsize(const void *ptr) } static int -wkmap_compact_table_i(st_data_t key, st_data_t val_obj, st_data_t data, int _error) +wkmap_compact_table_i(st_data_t key, st_data_t val, st_data_t _data, int _error) { - VALUE **dead_entry = (VALUE **)data; - ruby_sized_xfree(*dead_entry, sizeof(VALUE)); - *dead_entry = NULL; - - VALUE *key_ptr = (VALUE *)key; - - if (wmap_live_p(*key_ptr)) { - if (*key_ptr != rb_gc_location(*key_ptr) || val_obj != rb_gc_location(val_obj)) { - return ST_REPLACE; - } - - return ST_CONTINUE; + if ((VALUE)key != rb_gc_location((VALUE)key) || (VALUE)val != rb_gc_location((VALUE)val)) { + return ST_REPLACE; } - else { - *dead_entry = key_ptr; - return ST_DELETE; - } + return ST_CONTINUE; } static int @@ -728,7 +572,7 @@ wkmap_compact_table_replace(st_data_t *key_ptr, st_data_t *val_ptr, st_data_t _d { RUBY_ASSERT(existing); - *(VALUE *)*key_ptr = rb_gc_location(*(VALUE *)*key_ptr); + *key_ptr = (st_data_t)rb_gc_location((VALUE)*key_ptr); *val_ptr = (st_data_t)rb_gc_location((VALUE)*val_ptr); return ST_CONTINUE; @@ -740,15 +584,11 @@ wkmap_compact(void *ptr) struct weakkeymap *w = ptr; if (w->table) { - VALUE *dead_entry = NULL; - st_foreach_with_replace(w->table, wkmap_compact_table_i, wkmap_compact_table_replace, (st_data_t)&dead_entry); - if (dead_entry != NULL) { - ruby_sized_xfree(dead_entry, sizeof(VALUE)); - } + st_foreach_with_replace(w->table, wkmap_compact_table_i, wkmap_compact_table_replace, (st_data_t)0); } } -static const rb_data_type_t weakkeymap_type = { +const rb_data_type_t rb_weakkeymap_type = { "weakkeymap", { wkmap_mark, @@ -762,23 +602,16 @@ static const rb_data_type_t weakkeymap_type = { static int wkmap_cmp(st_data_t x, st_data_t y) { - VALUE x_obj = *(VALUE *)x; - VALUE y_obj = *(VALUE *)y; + VALUE x_obj = (VALUE)x; + VALUE y_obj = (VALUE)y; - if (wmap_live_p(x_obj) && wmap_live_p(y_obj)) { - return rb_any_cmp(x_obj, y_obj); - } - else { - /* If one of the objects is dead, then they cannot be the same. */ - return 1; - } + return rb_any_cmp(x_obj, y_obj); } static st_index_t wkmap_hash(st_data_t n) { - VALUE obj = *(VALUE *)n; - RUBY_ASSERT(wmap_live_p(obj)); + VALUE obj = (VALUE)n; return rb_any_hash(obj); } @@ -788,12 +621,37 @@ static const struct st_hash_type wkmap_hash_type = { wkmap_hash, }; +static int +rb_wkmap_handle_weak_references_i(st_data_t key, st_data_t val, st_data_t arg) +{ + if (rb_gc_handle_weak_references_alive_p(key)) { + return ST_CONTINUE; + } + else { + return ST_DELETE; + } +} + +void +rb_wkmap_handle_weak_references(VALUE self) +{ + struct weakkeymap *w; + TypedData_Get_Struct(self, struct weakkeymap, &rb_weakkeymap_type, w); + + st_foreach(w->table, rb_wkmap_handle_weak_references_i, (st_data_t)0); +} + static VALUE wkmap_allocate(VALUE klass) { struct weakkeymap *w; - VALUE obj = TypedData_Make_Struct(klass, struct weakkeymap, &weakkeymap_type, w); + + VALUE obj = TypedData_Make_Struct(klass, struct weakkeymap, &rb_weakkeymap_type, w); + w->table = st_init_table(&wkmap_hash_type); + + rb_gc_declare_weak_references(obj); + return obj; } @@ -801,10 +659,10 @@ static VALUE wkmap_lookup(VALUE self, VALUE key) { struct weakkeymap *w; - TypedData_Get_Struct(self, struct weakkeymap, &weakkeymap_type, w); + TypedData_Get_Struct(self, struct weakkeymap, &rb_weakkeymap_type, w); st_data_t data; - if (!st_lookup(w->table, (st_data_t)&key, &data)) return Qundef; + if (!st_lookup(w->table, (st_data_t)key, &data)) return Qundef; return (VALUE)data; } @@ -829,21 +687,6 @@ struct wkmap_aset_args { VALUE new_val; }; -static int -wkmap_aset_replace(st_data_t *key, st_data_t *val, st_data_t data_args, int existing) -{ - struct wkmap_aset_args *args = (struct wkmap_aset_args *)data_args; - - if (!existing) { - *key = (st_data_t)xmalloc(sizeof(VALUE)); - } - - *(VALUE *)*key = args->new_key; - *val = (st_data_t)args->new_val; - - return ST_CONTINUE; -} - /* * call-seq: * map[key] = value -> value @@ -860,19 +703,14 @@ static VALUE wkmap_aset(VALUE self, VALUE key, VALUE val) { struct weakkeymap *w; - TypedData_Get_Struct(self, struct weakkeymap, &weakkeymap_type, w); + TypedData_Get_Struct(self, struct weakkeymap, &rb_weakkeymap_type, w); if (!FL_ABLE(key) || SYMBOL_P(key) || RB_BIGNUM_TYPE_P(key) || RB_TYPE_P(key, T_FLOAT)) { rb_raise(rb_eArgError, "WeakKeyMap keys must be garbage collectable"); UNREACHABLE_RETURN(Qnil); } - struct wkmap_aset_args args = { - .new_key = key, - .new_val = val, - }; - - st_update(w->table, (st_data_t)&key, wkmap_aset_replace, (st_data_t)&args); + st_insert(w->table, (st_data_t)key, (st_data_t)val); RB_OBJ_WRITTEN(self, Qundef, key); RB_OBJ_WRITTEN(self, Qundef, val); @@ -913,19 +751,12 @@ static VALUE wkmap_delete(VALUE self, VALUE key) { struct weakkeymap *w; - TypedData_Get_Struct(self, struct weakkeymap, &weakkeymap_type, w); - - VALUE orig_key = key; - st_data_t orig_key_data = (st_data_t)&orig_key; - st_data_t orig_val_data; - if (st_delete(w->table, &orig_key_data, &orig_val_data)) { - VALUE orig_val = (VALUE)orig_val_data; - - rb_gc_remove_weak(self, (VALUE *)orig_key_data); - - ruby_sized_xfree((VALUE *)orig_key_data, sizeof(VALUE)); + TypedData_Get_Struct(self, struct weakkeymap, &rb_weakkeymap_type, w); - return orig_val; + st_data_t orig_key = (st_data_t)key; + st_data_t orig_val; + if (st_delete(w->table, &orig_key, &orig_val)) { + return (VALUE)orig_val; } if (rb_block_given_p()) { @@ -959,12 +790,12 @@ static VALUE wkmap_getkey(VALUE self, VALUE key) { struct weakkeymap *w; - TypedData_Get_Struct(self, struct weakkeymap, &weakkeymap_type, w); + TypedData_Get_Struct(self, struct weakkeymap, &rb_weakkeymap_type, w); st_data_t orig_key; - if (!st_get_key(w->table, (st_data_t)&key, &orig_key)) return Qnil; + if (!st_get_key(w->table, (st_data_t)key, &orig_key)) return Qnil; - return *(VALUE *)orig_key; + return (VALUE)orig_key; } /* @@ -979,17 +810,6 @@ wkmap_has_key(VALUE self, VALUE key) return RBOOL(!UNDEF_P(wkmap_lookup(self, key))); } -static int -wkmap_clear_i(st_data_t key, st_data_t val, st_data_t data) -{ - VALUE self = (VALUE)data; - - /* This WeakKeyMap may have already been marked, so we need to remove the - * keys to prevent a use-after-free. */ - rb_gc_remove_weak(self, (VALUE *)key); - return wkmap_free_table_i(key, val, 0); -} - /* * call-seq: * map.clear -> self @@ -1000,9 +820,8 @@ static VALUE wkmap_clear(VALUE self) { struct weakkeymap *w; - TypedData_Get_Struct(self, struct weakkeymap, &weakkeymap_type, w); + TypedData_Get_Struct(self, struct weakkeymap, &rb_weakkeymap_type, w); - st_foreach(w->table, wkmap_clear_i, (st_data_t)self); st_clear(w->table); return self; @@ -1023,7 +842,7 @@ static VALUE wkmap_inspect(VALUE self) { struct weakkeymap *w; - TypedData_Get_Struct(self, struct weakkeymap, &weakkeymap_type, w); + TypedData_Get_Struct(self, struct weakkeymap, &rb_weakkeymap_type, w); st_index_t n = st_table_size(w->table); diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 952cf88c205115..c081bc9e22abab 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -229,7 +229,7 @@ pub const RUBY_FL_TAINT: ruby_fl_type = 0; pub const RUBY_FL_EXIVAR: ruby_fl_type = 0; pub const RUBY_FL_SHAREABLE: ruby_fl_type = 256; pub const RUBY_FL_UNTRUSTED: ruby_fl_type = 0; -pub const RUBY_FL_UNUSED9: ruby_fl_type = 512; +pub const RUBY_FL_WEAK_REFERENCE: ruby_fl_type = 512; pub const RUBY_FL_UNUSED10: ruby_fl_type = 1024; pub const RUBY_FL_FREEZE: ruby_fl_type = 2048; pub const RUBY_FL_USER0: ruby_fl_type = 4096; diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 2689ec30cf8b2d..8f5ea5b8a837cd 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -290,7 +290,7 @@ pub const RUBY_FL_TAINT: ruby_fl_type = 0; pub const RUBY_FL_EXIVAR: ruby_fl_type = 0; pub const RUBY_FL_SHAREABLE: ruby_fl_type = 256; pub const RUBY_FL_UNTRUSTED: ruby_fl_type = 0; -pub const RUBY_FL_UNUSED9: ruby_fl_type = 512; +pub const RUBY_FL_WEAK_REFERENCE: ruby_fl_type = 512; pub const RUBY_FL_UNUSED10: ruby_fl_type = 1024; pub const RUBY_FL_FREEZE: ruby_fl_type = 2048; pub const RUBY_FL_USER0: ruby_fl_type = 4096;