Hi,

At Wed, 3 Sep 2008 07:10:24 +0900,
Joe Damato wrote in [ruby-core:18444]:
> with the patch applied:
> 
> [joe@mawu:/home/joe]% strace -ttT ruby -e 't1 = Thread.new { sleep(10) 
> }; t1.join; 10000.times { "aaaaaaaa" * 1000 };'  2>&1 | egrep 
> '(sigret|setitimer|timer|exit_group)'
> 14:29:17.536727 setitimer(ITIMER_VIRTUAL, {it_interval={0, 10000}, 
> it_value={0, 10000}}, NULL) = 0 <0.000006>
> 14:29:27.544435 setitimer(ITIMER_VIRTUAL, {it_interval={0, 0}, 
> it_value={0, 0}}, NULL) = 0 <0.000006>
> 14:29:27.948804 exit_group(0)           = ?
> 
> 
> Comments, suggestions, and questions are welcome.

Your patch ignores pthread.


Index: eval.c =================================================================== --- eval.c (revision 19075) +++ eval.c (working copy) @@ -10811,4 +10811,9 @@ rb_thread_remove(th) th->prev->next = th->next; th->next->prev = th->prev; + + /* if this is the last ruby thread, stop timer signals */ + if (th->next == th->prev && th->next == main_thread) { + rb_thread_stop_timer(); + } } @@ -12123,5 +12128,27 @@ catch_timer(sig) } +#define cleanup_begin(v, t, i, d) \ + pthread_cleanup_push((void (*)_((void *)))pthread_##t##_##d, v);\ + pthread_##t##_##i +#define cleanup_end() pthread_cleanup_pop(1) + +#define PER_NANO 1000000000 + +static struct timespec * +nsec_future(struct timespec *to, long ns) +{ + struct timeval tv; + gettimeofday(&tv, NULL); + to->tv_sec = tv.tv_sec; + to->tv_nsec = tv.tv_usec * 1000; + if ((to->tv_nsec += ns) >= PER_NANO) { + to->tv_sec += to->tv_nsec / PER_NANO; + to->tv_nsec %= PER_NANO; + } + return to; +} + static pthread_t time_thread; +static pthread_cond_t timer_running = PTHREAD_COND_INITIALIZER; static void* @@ -12129,4 +12156,9 @@ thread_timer(dummy) void *dummy; { + pthread_mutex_t lock; + pthread_cond_t *cond = dummy; + + struct timespec to; + sigset_t all_signals; @@ -12134,17 +12166,8 @@ thread_timer(dummy) pthread_sigmask(SIG_BLOCK, &all_signals, 0); - for (;;) { -#ifdef HAVE_NANOSLEEP - struct timespec req, rem; - - req.tv_sec = 0; - req.tv_nsec = 10000000; - nanosleep(&req, &rem); -#else - struct timeval tv; - tv.tv_sec = 0; - tv.tv_usec = 10000; - select(0, NULL, NULL, NULL, &tv); -#endif + cleanup_begin(&lock, mutex, init, destroy)(&lock, NULL); + cleanup_begin(&lock, mutex, lock, unlock)(&lock); + + while (pthread_cond_timedwait(cond, &lock, nsec_future(&to, PER_NANO / 100))) { if (!rb_thread_critical) { rb_thread_pending = 1; @@ -12154,4 +12177,9 @@ thread_timer(dummy) } } + + cleanup_end(); + cleanup_end(); + + return NULL; } @@ -12159,4 +12187,8 @@ void rb_thread_start_timer() { + if (!thread_init) return; + pthread_create(&time_thread, 0, thread_timer, &timer_running); + pthread_atfork(0, 0, rb_thread_stop_timer); + thread_init = 1; } @@ -12164,4 +12196,8 @@ void rb_thread_stop_timer() { + if (!thread_init) return; + pthread_cond_signal(&timer_running); + pthread_join(time_thread, NULL); + thread_init = 0; } #elif defined(HAVE_SETITIMER) @@ -12184,9 +12220,10 @@ rb_thread_start_timer() struct itimerval tval; - if (!thread_init) return; + if (thread_init) return; tval.it_interval.tv_sec = 0; tval.it_interval.tv_usec = 10000; tval.it_value = tval.it_interval; setitimer(ITIMER_VIRTUAL, &tval, NULL); + thread_init = 1; } @@ -12201,4 +12238,5 @@ rb_thread_stop_timer() tval.it_value = tval.it_interval; setitimer(ITIMER_VIRTUAL, &tval, NULL); + thread_init = 0; } #else /* !(_THREAD_SAFE || HAVE_SETITIMER) */ @@ -12224,5 +12262,4 @@ rb_thread_start_0(fn, arg, th) if (!thread_init) { - thread_init = 1; #if defined(HAVE_SETITIMER) || defined(_THREAD_SAFE) #if defined(POSIX_SIGNAL) @@ -12232,10 +12269,6 @@ rb_thread_start_0(fn, arg, th) #endif -#ifdef _THREAD_SAFE - pthread_create(&time_thread, 0, thread_timer, 0); -#else rb_thread_start_timer(); #endif -#endif }
-- Nobu Nakada