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