Skip to content

Commit

Permalink
Fix #326. Crash was caused by rp_methods holding a reference to Profi…
Browse files Browse the repository at this point in the history
…le ruby objects which were moved during GC compaction phase.
  • Loading branch information
cfis committed Jan 7, 2024
1 parent 9930849 commit 3f088ea
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 13 deletions.
20 changes: 11 additions & 9 deletions ext/ruby_prof/rp_method.c
Expand Up @@ -4,6 +4,7 @@
#include "rp_allocation.h"
#include "rp_call_trees.h"
#include "rp_method.h"
#include "rp_profile.h"

VALUE cRpMethodInfo;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
Expand Down
7 changes: 5 additions & 2 deletions ext/ruby_prof/rp_method.h
Expand Up @@ -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

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions ext/ruby_prof/rp_profile.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down

0 comments on commit 3f088ea

Please sign in to comment.