Skip to content

Commit 1c4102e

Browse files
committed
Fix pending finalizer jobs on exit
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
1 parent a62dbe9 commit 1c4102e

File tree

3 files changed

+35
-8
lines changed

3 files changed

+35
-8
lines changed

Diff for: gc/default.c

+32-3
Original file line numberDiff line numberDiff line change
@@ -2998,6 +2998,10 @@ rb_mmtk_make_dfree_job(void (*dfree)(void *), void *data)
29982998
job->kind = MMTK_FJOB_DFREE;
29992999
job->as.dfree.dfree = dfree;
30003000
job->as.dfree.data = data;
3001+
3002+
RUBY_DEBUG_LOG("Created dfree job %p. dfree: %p, data: %p\n",
3003+
job, dfree, data);
3004+
30013005
rb_mmtk_push_final_job(job);
30023006
}
30033007

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

3018+
RUBY_DEBUG_LOG("Created finalize job %p. obj: %p, finalizer_array: %p\n",
3019+
job, (void*)obj, (void*)finalizer_array);
3020+
30143021
VALUE observed_id = Qnil;
30153022
if (FL_TEST(obj, FL_SEEN_OBJ_ID)) {
30163023
// obj is technically dead already,
@@ -3364,16 +3371,22 @@ rb_mmtk_run_final_job(struct MMTk_FinalJob *job)
33643371
{
33653372
switch (job->kind) {
33663373
case MMTK_FJOB_DFREE: {
3374+
RUBY_DEBUG_LOG("Running dfree job %p. dfree: %p, data: %p\n",
3375+
job, job->as.dfree.dfree, job->as.dfree.data);
33673376
job->as.dfree.dfree(job->as.dfree.data);
33683377
break;
33693378
}
33703379
case MMTK_FJOB_FINALIZE: {
3380+
VALUE objid = job->as.finalize.observed_id;
3381+
VALUE table = job->as.finalize.finalizer_array;
3382+
3383+
RUBY_DEBUG_LOG("Running finalize job %p. observed_id: %p, table: %p\n",
3384+
job, (void*)objid, (void*)table);
3385+
33713386
if (rb_gc_obj_free_on_exit_started()) {
33723387
rb_bug("Finalize job still exists after obj_free on exit has started.");
33733388
}
33743389

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

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

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

3612+
RUBY_ASSERT(finalizer_table->num_entries == 0);
3613+
RUBY_ASSERT(heap_pages_deferred_final == 0);
3614+
35933615
// Tell the world that obj_free on exit has started.
35943616
rb_gc_set_obj_free_on_exit_started();
35953617

@@ -10608,4 +10630,11 @@ rb_mmtk_newobj_raw(VALUE klass, VALUE flags, int wb_protected, size_t payload_si
1060810630
{
1060910631
return rb_mmtk_newobj_of_inner(klass, flags, wb_protected, payload_size);
1061010632
}
10633+
10634+
void
10635+
rb_mmtk_gc_finalize_deferred_register(void)
10636+
{
10637+
rb_objspace_t *objspace = rb_gc_get_objspace();
10638+
gc_finalize_deferred_register(objspace);
10639+
}
1061110640
#endif

Diff for: internal/gc.h

+1
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ void rb_mmtk_run_finalizer(VALUE obj, VALUE table);
235235
void rb_mmtk_set_during_gc(bool is_during_gc);
236236
void rb_mmtk_get_vanilla_times(uint64_t *mark, uint64_t *sweep);
237237
VALUE rb_mmtk_newobj_raw(VALUE klass, VALUE flags, int wb_protected, size_t payload_size);
238+
void rb_mmtk_gc_finalize_deferred_register(void);
238239
#endif
239240

240241
RUBY_SYMBOL_EXPORT_BEGIN

Diff for: mmtk_support.c

+2-5
Original file line numberDiff line numberDiff line change
@@ -1523,11 +1523,8 @@ rb_mmtk_block_for_gc(MMTk_VMMutatorThread tls)
15231523
RB_VM_SAVE_MACHINE_CONTEXT(cur_th);
15241524
rb_mmtk_use_mmtk_global(rb_mmtk_block_for_gc_internal, NULL);
15251525

1526-
#if USE_MMTK
1527-
if (rb_mmtk_enabled_p()) {
1528-
RUBY_DEBUG_LOG("GC finished. Mutator resumed.");
1529-
}
1530-
#endif
1526+
// Trigger postponed job so that a mutator will start running pending final jobs soon.
1527+
rb_mmtk_gc_finalize_deferred_register();
15311528
}
15321529

15331530
static void

0 commit comments

Comments
 (0)