From 3f088ea3f8548e2db1b2d9d990d051ff7c2a358a Mon Sep 17 00:00:00 2001 From: Charlie Savage Date: Sat, 6 Jan 2024 18:19:15 -0800 Subject: [PATCH] Fix #326. Crash was caused by rp_methods holding a reference to Profile ruby objects which were moved during GC compaction phase. --- ext/ruby_prof/rp_method.c | 20 +++++++++++--------- ext/ruby_prof/rp_method.h | 7 +++++-- ext/ruby_prof/rp_profile.c | 4 ++-- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/ext/ruby_prof/rp_method.c b/ext/ruby_prof/rp_method.c index 60bcf932..2630f41e 100644 --- a/ext/ruby_prof/rp_method.c +++ b/ext/ruby_prof/rp_method.c @@ -4,6 +4,7 @@ #include "rp_allocation.h" #include "rp_call_trees.h" #include "rp_method.h" +#include "rp_profile.h" VALUE cRpMethodInfo; @@ -125,7 +126,7 @@ prof_method_t* prof_get_method(VALUE self) return result; } -prof_method_t* prof_method_create(VALUE profile, VALUE klass, VALUE msym, VALUE source_file, int source_line) +prof_method_t* prof_method_create(struct prof_profile_t* profile, VALUE klass, VALUE msym, VALUE source_file, int source_line) { prof_method_t* result = ALLOC(prof_method_t); result->profile = profile; @@ -204,14 +205,16 @@ void prof_method_mark(void* data) prof_method_t* method = (prof_method_t*)data; - if (method->profile != Qnil) - rb_gc_mark_movable(method->profile); - if (method->object != Qnil) - rb_gc_mark_movable(method->object); + rb_gc_mark_movable(method->object); + + // Mark the profile to keep it alive. Can't call prof_profile_mark because that would + // cause recursion + if (method->profile && method->profile->object != Qnil) + rb_gc_mark(method->profile->object); - rb_gc_mark_movable(method->klass_name); - rb_gc_mark_movable(method->method_name); + rb_gc_mark(method->klass_name); + rb_gc_mark(method->method_name); rb_gc_mark(method->source_file); if (method->klass != Qnil) @@ -225,14 +228,13 @@ void prof_method_compact(void* data) { prof_method_t* method = (prof_method_t*)data; method->object = rb_gc_location(method->object); - method->profile = rb_gc_location(method->profile); method->klass_name = rb_gc_location(method->klass_name); method->method_name = rb_gc_location(method->method_name); } static VALUE prof_method_allocate(VALUE klass) { - prof_method_t* method_data = prof_method_create(Qnil, Qnil, Qnil, Qnil, 0); + prof_method_t* method_data = prof_method_create(NULL, Qnil, Qnil, Qnil, 0); method_data->object = prof_method_wrap(method_data); return method_data->object; } diff --git a/ext/ruby_prof/rp_method.h b/ext/ruby_prof/rp_method.h index e1b21d7f..ff526771 100644 --- a/ext/ruby_prof/rp_method.h +++ b/ext/ruby_prof/rp_method.h @@ -18,11 +18,14 @@ enum { kOtherSingleton = 0x10 // Singleton of unknown object }; +// Don't want to include ruby_prof.h to avoid a circular reference +struct prof_profile_t; + // Profiling information for each method. // Excluded methods have no call_trees, source_klass, or source_file. typedef struct prof_method_t { - VALUE profile; // Profile this method is associated with - needed for mark phase + struct prof_profile_t* profile; // Profile this method is associated with - needed for mark phase struct prof_call_trees_t* call_trees; // Call infos that call this method st_table* allocations_table; // Tracks object allocations @@ -51,7 +54,7 @@ prof_method_t* method_table_lookup(st_table* table, st_data_t key); size_t method_table_insert(st_table* table, st_data_t key, prof_method_t* val); void method_table_free(st_table* table); void prof_method_table_merge(st_table* self, st_table* other); -prof_method_t* prof_method_create(VALUE profile, VALUE klass, VALUE msym, VALUE source_file, int source_line); +prof_method_t* prof_method_create(struct prof_profile_t* profile, VALUE klass, VALUE msym, VALUE source_file, int source_line); prof_method_t* prof_get_method(VALUE self); VALUE prof_method_wrap(prof_method_t* result); diff --git a/ext/ruby_prof/rp_profile.c b/ext/ruby_prof/rp_profile.c index bd16404a..88728a10 100644 --- a/ext/ruby_prof/rp_profile.c +++ b/ext/ruby_prof/rp_profile.c @@ -105,7 +105,7 @@ static int excludes_method(st_data_t key, prof_profile_t* profile) static prof_method_t* create_method(prof_profile_t* profile, st_data_t key, VALUE klass, VALUE msym, VALUE source_file, int source_line) { - prof_method_t* result = prof_method_create(profile->object, klass, msym, source_file, source_line); + prof_method_t* result = prof_method_create(profile, klass, msym, source_file, source_line); method_table_insert(profile->last_thread_data->method_table, result->key, result); return result; @@ -863,7 +863,7 @@ static VALUE prof_exclude_method(VALUE self, VALUE klass, VALUE msym) if (!method) { - method = prof_method_create(self, klass, msym, Qnil, 0); + method = prof_method_create(profile, klass, msym, Qnil, 0); method_table_insert(profile->exclude_methods_tbl, method->key, method); }