[ruby-core:32724] Re: [Ruby 1.9-Feature#3436] Spawn the timer thread lazily

Hi,

At Tue, 5 Oct 2010 03:09:42 +0900,
Mark Somerville wrote in [ruby-core:32686]:
> The thread is now only used when it is required to schedule
> Ruby threads. When there is only the main thread, signals are
> handled immediately in sighandler().

Seems almost fine.  Since aded missing prototypes, no needs to
move some functions now.

> I only have access to Linux boxes. One of the added tests
> isn't used on other platforms.

Updated patch with new test.


diff --git c/ext/-test-/thread/timer/extconf.rb w/ext/-test-/thread/timer/extconf.rb
new file mode 100644
index 0000000..a28904b
--- /dev/null
+++ w/ext/-test-/thread/timer/extconf.rb
@@ -0,0 +1,3 @@
+require 'mkmf'
+$INCFLAGS << " -I$(top_srcdir)"
+create_makefile('-test-/thread/timer')
diff --git c/ext/-test-/thread/timer/timer.c w/ext/-test-/thread/timer/timer.c
new file mode 100644
index 0000000..6d77b1c
--- /dev/null
+++ w/ext/-test-/thread/timer/timer.c
@@ -0,0 +1,15 @@
+#include <ruby.h>
+#include "vm_core.h"
+
+VALUE
+thread_timer_thread_running_p(VALUE klass)
+{
+    return rb_thread_timer_thread_is_running() ? Qtrue : Qfalse;
+}
+
+void
+Init_timer(void)
+{
+    VALUE timer = rb_define_module_under(rb_cThread, "Timer");
+    rb_define_module_function(timer, "running?", thread_timer_thread_running_p, 0);
+}
diff --git c/process.c w/process.c
index 55b83b1..3a35725 100644
--- c/process.c
+++ w/process.c
@@ -998,7 +998,7 @@ static int forked_child = 0;
 #define before_exec() \
     (rb_enable_interrupt(), (void)(forked_child ? 0 : (rb_thread_stop_timer_thread(), 1)))
 #define after_exec() \
-  (rb_thread_reset_timer_thread(), rb_thread_start_timer_thread(), forked_child = 0, rb_disable_interrupt())
+  (rb_thread_reset_timer_thread(), rb_thread_start_timer_thread(), forked_child = 0)
 #define before_fork() before_exec()
 #define after_fork() (GET_THREAD()->thrown_errinfo = 0, after_exec())
 
diff --git c/signal.c w/signal.c
index ba35954..ce44d81 100644
--- c/signal.c
+++ w/signal.c
@@ -519,6 +519,10 @@ sighandler(int sig)
 #if !defined(BSD_SIGNAL) && !defined(POSIX_SIGNAL)
     ruby_signal(sig, sighandler);
 #endif
+
+    if (rb_thread_alone()) {
+	rb_threadptr_check_signal(GET_THREAD());
+    }
 }
 
 int
diff --git c/test/ruby/test_signal.rb w/test/ruby/test_signal.rb
index 0098ccc..6371d8b 100644
--- c/test/ruby/test_signal.rb
+++ w/test/ruby/test_signal.rb
@@ -181,4 +181,20 @@ class TestSignal < Test::Unit::TestCase
     w.close
     assert_equal(r.read, "foo")
   end
+
+  def test_signals_before_and_after_timer_thread
+    count = 0
+    Signal.trap(:INT) { count += 1 }
+
+    Process.kill :INT, Process.pid
+    assert_equal 1, count
+
+    th = Thread.new { sleep 0.5 }
+    Process.kill :INT, Process.pid
+    assert_equal 2, count
+
+    th.join
+    Process.kill :INT, Process.pid
+    assert_equal 3, count
+  end
 end
diff --git c/test/thread/test_timer_thread.rb w/test/thread/test_timer_thread.rb
new file mode 100644
index 0000000..b732aa0
--- /dev/null
+++ w/test/thread/test_timer_thread.rb
@@ -0,0 +1,10 @@
+require 'test/unit'
+require '-test-/thread/timer'
+
+class TestTimerThread < Test::Unit::TestCase
+  def test_timer_is_created_and_destroyed
+    assert !Thread::Timer.running?
+    assert Thread.new {Thread::Timer.running?}.value
+    assert !Thread::Timer.running?
+  end
+end
diff --git c/thread.c w/thread.c
index 11c87be..471140e 100644
--- c/thread.c
+++ w/thread.c
@@ -562,6 +562,7 @@ thread_create_core(VALUE thval, VALUE args, VALUE (*fn)(ANYARGS))
 	th->status = THREAD_KILLED;
 	rb_raise(rb_eThreadError, "can't create Thread (%d)", err);
     }
+    rb_thread_start_timer_thread();
     return thval;
 }
 
@@ -577,6 +578,7 @@ thread_s_new(int argc, VALUE *argv, VALUE klass)
 	rb_raise(rb_eThreadError, "uninitialized thread - check `%s#initialize'",
 		 rb_class2name(klass));
     }
+    rb_thread_start_timer_thread();
     return thread;
 }
 
@@ -711,6 +713,12 @@ thread_join(rb_thread_t *target_th, double delay)
     thread_debug("thread_join: success (thid: %p)\n",
 		 (void *)target_th->thread_id);
 
+    rb_disable_interrupt();
+    if (rb_thread_alone() && rb_signal_buff_size() == 0) {
+	rb_thread_stop_timer_thread();
+    }
+    rb_enable_interrupt();
+
     if (target_th->errinfo != Qnil) {
 	VALUE err = target_th->errinfo;
 
@@ -2723,7 +2731,15 @@ void
 rb_thread_start_timer_thread(void)
 {
     system_working = 1;
-    rb_thread_create_timer_thread();
+    if (!rb_thread_alone()) {
+	rb_thread_create_timer_thread();
+    }
+}
+
+int
+rb_thread_timer_thread_is_running(void)
+{
+    return native_timer_thread_is_running();
 }
 
 static int
@@ -4262,7 +4278,7 @@ Init_Thread(void)
 	}
     }
 
-    rb_thread_create_timer_thread();
+    rb_thread_start_timer_thread();
 
     (void)native_mutex_trylock;
 }
diff --git c/thread_pthread.c w/thread_pthread.c
index e835bf8..b229b32 100644
--- c/thread_pthread.c
+++ w/thread_pthread.c
@@ -840,6 +840,12 @@ native_reset_timer_thread(void)
     timer_thread_id = 0;
 }
 
+static int
+native_timer_thread_is_running(void)
+{
+    return timer_thread_id != 0;
+}
+
 #ifdef HAVE_SIGALTSTACK
 int
 ruby_stack_overflowed_p(const rb_thread_t *th, const void *addr)
diff --git c/thread_win32.c w/thread_win32.c
index 9e64ea4..e52f8de 100644
--- c/thread_win32.c
+++ w/thread_win32.c
@@ -590,4 +590,10 @@ native_reset_timer_thread(void)
     }
 }
 
+static int
+native_timer_thread_is_running(void)
+{
+    return timer_thread_id != 0;
+}
+
 #endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */
diff --git c/vm_core.h w/vm_core.h
index e43f539..7dc4b8d 100644
--- c/vm_core.h
+++ w/vm_core.h
@@ -635,6 +635,7 @@ VALUE rb_iseq_eval(VALUE iseqval);
 VALUE rb_iseq_eval_main(VALUE iseqval);
 void rb_enable_interrupt(void);
 void rb_disable_interrupt(void);
+int rb_thread_timer_thread_is_running(void);
 #if defined __GNUC__ && __GNUC__ >= 4
 #pragma GCC visibility pop
 #endif


-- 
Nobu Nakada