Skip to content

Commit

Permalink
Fix pending finalizer jobs on exit
Browse files Browse the repository at this point in the history
If a GC is triggered by allocation, we didn't execute final jobs until
exit.  This is a bug.  If there are pending finalizer jobs while the
finalizer_table is empty on exit, it will trigger an assertion error.

Now we execute finalizer jobs more eagerly.

-   Like the default GC, we trigger postponed job after GC in order to
    run final jobs promptly.
-   On exit, we ensure both finalizer_table and pending final jobs are
    drained before starting to force obj_free.

Fixes: #81
  • Loading branch information
wks committed Aug 21, 2024
1 parent a62dbe9 commit 1c4102e
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 8 deletions.
35 changes: 32 additions & 3 deletions gc/default.c
Original file line number Diff line number Diff line change
Expand Up @@ -2998,6 +2998,10 @@ rb_mmtk_make_dfree_job(void (*dfree)(void *), void *data)
job->kind = MMTK_FJOB_DFREE;
job->as.dfree.dfree = dfree;
job->as.dfree.data = data;

RUBY_DEBUG_LOG("Created dfree job %p. dfree: %p, data: %p\n",
job, dfree, data);

rb_mmtk_push_final_job(job);
}

Expand All @@ -3011,6 +3015,9 @@ rb_mmtk_make_finalize_job(VALUE obj, VALUE finalizer_array)
struct MMTk_FinalJob *job = (struct MMTk_FinalJob*)xmalloc(sizeof(struct MMTk_FinalJob));
job->kind = MMTK_FJOB_FINALIZE;

RUBY_DEBUG_LOG("Created finalize job %p. obj: %p, finalizer_array: %p\n",
job, (void*)obj, (void*)finalizer_array);

VALUE observed_id = Qnil;
if (FL_TEST(obj, FL_SEEN_OBJ_ID)) {
// obj is technically dead already,
Expand Down Expand Up @@ -3364,16 +3371,22 @@ rb_mmtk_run_final_job(struct MMTk_FinalJob *job)
{
switch (job->kind) {
case MMTK_FJOB_DFREE: {
RUBY_DEBUG_LOG("Running dfree job %p. dfree: %p, data: %p\n",
job, job->as.dfree.dfree, job->as.dfree.data);
job->as.dfree.dfree(job->as.dfree.data);
break;
}
case MMTK_FJOB_FINALIZE: {
VALUE objid = job->as.finalize.observed_id;
VALUE table = job->as.finalize.finalizer_array;

RUBY_DEBUG_LOG("Running finalize job %p. observed_id: %p, table: %p\n",
job, (void*)objid, (void*)table);

if (rb_gc_obj_free_on_exit_started()) {
rb_bug("Finalize job still exists after obj_free on exit has started.");
}

VALUE objid = job->as.finalize.observed_id;
VALUE table = job->as.finalize.finalizer_array;
rb_gc_run_obj_finalizer(objid, RARRAY_LEN(table), get_final, (void *)table);

break;
Expand Down Expand Up @@ -3580,7 +3593,13 @@ rb_gc_impl_shutdown_call_finalizer(void *objspace_ptr)
#if USE_MMTK
if (rb_mmtk_enabled_p()) {
// Force to run finalizers, the MMTk style.
while (finalizer_table->num_entries) {
// We repeatedly vacate the finalizer table and run final jobs
// until the finalizer table is empty and there are no pending final jobs
// Note: Final jobs are executed immediately after `GC.start`,
// and are also executed when interrupted after a GC triggered by allocation.
// Just in case the VM exits before the interrupts are handled,
// we explicitly drain the pending final jobs here.
while (finalizer_table->num_entries || heap_pages_deferred_final != 0) {
// We move all elements from the finalizer_table to heap_pages_deferred_final.
st_foreach(finalizer_table, rb_mmtk_evacuate_finalizer_table_on_exit_i, 0);

Expand All @@ -3590,6 +3609,9 @@ rb_gc_impl_shutdown_call_finalizer(void *objspace_ptr)
// We need to repeat because a finalizer may register new finalizers.
}

RUBY_ASSERT(finalizer_table->num_entries == 0);
RUBY_ASSERT(heap_pages_deferred_final == 0);

// Tell the world that obj_free on exit has started.
rb_gc_set_obj_free_on_exit_started();

Expand Down Expand Up @@ -10608,4 +10630,11 @@ rb_mmtk_newobj_raw(VALUE klass, VALUE flags, int wb_protected, size_t payload_si
{
return rb_mmtk_newobj_of_inner(klass, flags, wb_protected, payload_size);
}

void
rb_mmtk_gc_finalize_deferred_register(void)
{
rb_objspace_t *objspace = rb_gc_get_objspace();
gc_finalize_deferred_register(objspace);
}
#endif
1 change: 1 addition & 0 deletions internal/gc.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ void rb_mmtk_run_finalizer(VALUE obj, VALUE table);
void rb_mmtk_set_during_gc(bool is_during_gc);
void rb_mmtk_get_vanilla_times(uint64_t *mark, uint64_t *sweep);
VALUE rb_mmtk_newobj_raw(VALUE klass, VALUE flags, int wb_protected, size_t payload_size);
void rb_mmtk_gc_finalize_deferred_register(void);
#endif

RUBY_SYMBOL_EXPORT_BEGIN
Expand Down
7 changes: 2 additions & 5 deletions mmtk_support.c
Original file line number Diff line number Diff line change
Expand Up @@ -1523,11 +1523,8 @@ rb_mmtk_block_for_gc(MMTk_VMMutatorThread tls)
RB_VM_SAVE_MACHINE_CONTEXT(cur_th);
rb_mmtk_use_mmtk_global(rb_mmtk_block_for_gc_internal, NULL);

#if USE_MMTK
if (rb_mmtk_enabled_p()) {
RUBY_DEBUG_LOG("GC finished. Mutator resumed.");
}
#endif
// Trigger postponed job so that a mutator will start running pending final jobs soon.
rb_mmtk_gc_finalize_deferred_register();
}

static void
Expand Down

0 comments on commit 1c4102e

Please sign in to comment.