[ruby-core:37205] Re: [Request for Comment] avoid timer thread

Eric Wong <[e-mail]> wrote:
> I've even switched my Rainbows! instance (it calls fork+exec cgit.cgi)
> over to running Ruby trunk + your patch:

I found a race condition in my server that calls fork+exec
often:
  unicorn-3.7.0/lib/unicorn/app/exec_cgi.rb:66: [BUG] rb_thread_wakeup_timer_thread - write: Broken pipe (EPIPE)

exec_cgi.rb code is here, the fork { exec } is all in Ruby:
http://bogomips.org/unicorn.git/tree/lib/unicorn/app/exec_cgi.rb?id=v3.7.0

I've only seen this happen once out of 8000 or so requests (mostly search
spiders).  This process (Rainbows!) is also configured to use
multithreading, so there could be many things happening that could
fire a signal...

I'm testing the following fix on my server:
http://bogomips.org/ruby.git/commit/?h=sleepy-timer-thread&id=9154363119

I'm also sending SIGUSR1 every minute to increase the chance of hitting
this error and telling robots to crawl faster.

From 9154363119cd09a3b79aa45753d177ec65914f7c Mon Sep 17 00:00:00 2001
From: Eric Wong <[e-mail]>
Date: Fri, 17 Jun 2011 19:52:48 +0000
Subject: [PATCH] defer timer thread wakeups after fork()

There is a small race condition where a signal can be received
immediately after fork() but before the timer thread pipe can be
reinitialized, leading to EPIPE on write().

We probably can't disable interrupt before forking due to
system/exec() issues; and we don't want to lose wakeups to the
parent process, so we check the cached pid vs getpid() whenever
rb_thread_wakeup_timer_thread() is called.
---
 thread_pthread.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/thread_pthread.c b/thread_pthread.c
index 1cc212b..3d99974 100644
--- a/thread_pthread.c
+++ b/thread_pthread.c
@@ -989,11 +989,18 @@ static void ping_signal_thread_list(void) { }
 #endif /* USE_SIGNAL_THREAD_LIST */
 
 static pthread_t timer_thread_id;
+static sig_atomic_t timer_thread_deferred;
+static pid_t timer_thread_pid;
 static int timer_thread_pipe[2];
 
 void
 rb_thread_wakeup_timer_thread(void)
 {
+    if (getpid() != timer_thread_pid) {
+	timer_thread_deferred = 1;
+	return;
+    }
+
     /* already opened */
     if (timer_thread_id && timer_thread_pipe[1]) {
 	const char *buff = "!";
@@ -1021,6 +1028,12 @@ thread_timer(void *p)
 
     if (TT_DEBUG) fprintf(stderr, "start timer thread\n");
 
+    if (timer_thread_deferred) {
+	timer_thread_deferred = 0;
+	ping_signal_thread_list();
+	timer_thread_function(0);
+    }
+
     while (system_working > 0) {
 	int err;
 
@@ -1093,6 +1106,7 @@ rb_thread_create_timer_thread(void)
 	    fprintf(stderr, "[FATAL] Failed to create timer thread (errno: %d)\n", err);
 	    exit(EXIT_FAILURE);
 	}
+	timer_thread_pid = getpid();
     }
     rb_disable_interrupt(); /* only timer thread recieve signal */
 }
@@ -1118,6 +1132,7 @@ native_reset_timer_thread(void)
 {
     if (TT_DEBUG)  fprintf(stderr, "reset timer thread\n");
     timer_thread_id = 0;
+    timer_thread_pid = 0;
 
     /* close */
     if (timer_thread_pipe[0] && close(timer_thread_pipe[0]) < 0) { rb_bug_errno("native_stop_timer_thread - close(ttp[0])", errno); }
-- 
Eric Wong