From 415ee286894b96263ab82d4e06b7e4a8398b3544 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 22 Apr 2021 14:22:45 -0700 Subject: [PATCH 1/3] Remove reentrancy detection from postponed job Because this is in the postponed job handler, we should never be able to reenter this code. I also believe this has a race condition if another signal arrives between the if statement and the increment. Signals can also arrive in any thread so this would not be a safe way to avoid reentrancy. --- ext/stackprof/stackprof.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/ext/stackprof/stackprof.c b/ext/stackprof/stackprof.c index a65531b..e4fb1d6 100644 --- a/ext/stackprof/stackprof.c +++ b/ext/stackprof/stackprof.c @@ -596,25 +596,17 @@ stackprof_record_gc_samples() static void stackprof_gc_job_handler(void *data) { - static int in_signal_handler = 0; - if (in_signal_handler) return; if (!_stackprof.running) return; - in_signal_handler++; stackprof_record_gc_samples(); - in_signal_handler--; } static void stackprof_job_handler(void *data) { - static int in_signal_handler = 0; - if (in_signal_handler) return; if (!_stackprof.running) return; - in_signal_handler++; stackprof_record_sample(); - in_signal_handler--; } static void From cf67aff9cf7c7725c9f0b4ca3010a9223753ed67 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 28 Apr 2021 13:21:04 -0700 Subject: [PATCH 2/3] Use a mutex to avoid reentering signal handler This entures we won't re-enter the signal handler from another signal whether or not it happens in our own thread. --- ext/stackprof/stackprof.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ext/stackprof/stackprof.c b/ext/stackprof/stackprof.c index e4fb1d6..7a0acca 100644 --- a/ext/stackprof/stackprof.c +++ b/ext/stackprof/stackprof.c @@ -612,7 +612,14 @@ stackprof_job_handler(void *data) static void stackprof_signal_handler(int sig, siginfo_t *sinfo, void *ucontext) { + static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; + _stackprof.overall_signals++; + + if (!_stackprof.running) return; + if (!ruby_native_thread_p()) return; + if (pthread_mutex_trylock(&lock)) return; + if (!_stackprof.ignore_gc && rb_during_gc()) { VALUE mode = rb_gc_latest_gc_info(sym_state); if (mode == sym_marking) { @@ -625,6 +632,7 @@ stackprof_signal_handler(int sig, siginfo_t *sinfo, void *ucontext) } else { rb_postponed_job_register_one(0, stackprof_job_handler, (void*)0); } + pthread_mutex_unlock(&lock); } static void From 6c3f4d7c5234cb3274c4a624ea6cb52c26be3833 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 28 Apr 2021 13:19:07 -0700 Subject: [PATCH 3/3] Collect stack frames immediately on interrupt Co-authored-by: Aaron Patterson --- ext/stackprof/stackprof.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ext/stackprof/stackprof.c b/ext/stackprof/stackprof.c index 7a0acca..b1dfd86 100644 --- a/ext/stackprof/stackprof.c +++ b/ext/stackprof/stackprof.c @@ -22,6 +22,14 @@ #define FAKE_FRAME_MARK INT2FIX(1) #define FAKE_FRAME_SWEEP INT2FIX(2) +/* + * As of Ruby 3.0, it should be safe to read stack frames at any time + * See https://github.com/ruby/ruby/commit/0e276dc458f94d9d79a0f7c7669bde84abe80f21 + */ +#if RUBY_API_VERSION_MAJOR < 3 + #define USE_POSTPONED_JOB +#endif + static const char *fake_frame_cstrs[] = { "(garbage collection)", "(marking)", @@ -630,7 +638,11 @@ stackprof_signal_handler(int sig, siginfo_t *sinfo, void *ucontext) _stackprof.unrecorded_gc_samples++; rb_postponed_job_register_one(0, stackprof_gc_job_handler, (void*)0); } else { +#ifdef USE_POSTPONED_JOB rb_postponed_job_register_one(0, stackprof_job_handler, (void*)0); +#else + stackprof_job_handler(0); +#endif } pthread_mutex_unlock(&lock); }