From 66f3b6c1b043b814663668b5f83210c6e8d1e12d Mon Sep 17 00:00:00 2001 From: Neil Jerram Date: Wed, 20 May 2009 21:55:35 +0100 Subject: [PATCH] Remove possible deadlock in scm_join_thread * libguile/threads.c (scm_join_thread): Always recheck t->exited before calling block_self again, in case thread t has now exited. * test-suite/tests/threads.test (joining): New test. --- libguile/threads.c | 17 +++++++---------- test-suite/tests/threads.test | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/libguile/threads.c b/libguile/threads.c index fc3e607..3d6df11 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -934,17 +934,14 @@ SCM_DEFINE (scm_join_thread, "join-thread", 1, 0, 0, scm_i_scm_pthread_mutex_lock (&thread_admin_mutex); t = SCM_I_THREAD_DATA (thread); - if (!t->exited) + while (!t->exited) { - while (1) - { - block_self (t->join_queue, thread, &thread_admin_mutex, NULL); - if (t->exited) - break; - scm_i_pthread_mutex_unlock (&thread_admin_mutex); - SCM_TICK; - scm_i_scm_pthread_mutex_lock (&thread_admin_mutex); - } + block_self (t->join_queue, thread, &thread_admin_mutex, NULL); + if (t->exited) + break; + scm_i_pthread_mutex_unlock (&thread_admin_mutex); + SCM_TICK; + scm_i_scm_pthread_mutex_lock (&thread_admin_mutex); } res = t->result; diff --git a/test-suite/tests/threads.test b/test-suite/tests/threads.test index 0146016..34ee7ee 100644 --- a/test-suite/tests/threads.test +++ b/test-suite/tests/threads.test @@ -133,4 +133,36 @@ (lambda (n) (set! result (cons n result))) (lambda (n) (* 2 n)) '(0 1 2 3 4 5)) - (equal? result '(10 8 6 4 2 0))))))) + (equal? result '(10 8 6 4 2 0))))) + + ;; + ;; thread joining + ;; + + (with-test-prefix "joining" + + ;; scm_join_thread has a SCM_TICK in the middle of it, to + ;; allow asyncs to run (including signal delivery). We used + ;; to have a bug whereby if the joined thread terminated at + ;; the same time as the joining thread is in this SCM_TICK, + ;; scm_join_thread would not notice and would hang forever. + ;; So in this test we are setting up the following sequence of + ;; events. + ;; T=0 other thread is created and starts running + ;; T=2 main thread sets up an async that will sleep for 10 seconds + ;; T=2 main thread calls join-thread, which will... + ;; T=2 ...call the async, which starts sleeping + ;; T=5 other thread finishes its work and terminates + ;; T=7 async completes, main thread continues inside join-thread. + (pass-if "don't hang when joined thread terminates in SCM_TICK" + (let ((other-thread (make-thread sleep 5))) + (letrec ((delay-count 10) + (aproc (lambda () + (set! delay-count (- delay-count 1)) + (if (zero? delay-count) + (sleep 5) + (system-async-mark aproc))))) + (sleep 2) + (system-async-mark aproc) + (join-thread other-thread))) + #t)))) -- 1.5.6.5