diff options
author | Ludovic Courtès <ludo@gnu.org> | 2010-09-02 14:42:14 +0200 |
---|---|---|
committer | Ludovic Courtès <ludo@gnu.org> | 2010-09-02 14:42:14 +0200 |
commit | f57fdf07d6374028f35bcb1ee748a94022deda6d (patch) | |
tree | 613b1be8bd4a59e8ffa9477667c2c07aa3aff714 | |
parent | d31ae2c363cf92b62260e9513534c7c046a4315f (diff) | |
download | guile-f57fdf07d6374028f35bcb1ee748a94022deda6d.tar.gz |
Fix memory leak in `lock-mutex' (aka. `scm_lock_mutex'.)
The memory leak is trivially reproducible with:
(define m (make-mutex))
(let loop () (lock-mutex m) (unlock-mutex m) (loop))
or similarly with:
(define p (delay (+ 1 2)))
(let loop () (force p) (loop))
since `force' acquires P's mutex.
It could also lead to premature release of a thread waiting in
`fat_mutex_lock' when a former owner's `do_thread_exit' is run.
* libguile/threads.c (fat_mutex_unlock): When `m->level' becomes 0,
remove MUTEX from `t->mutexes'.
(fat_mutex_lock): Update comment above the `t->mutexes' assignment.
(do_thread_exit): Add an assertion making sure that each mutex in
`t->mutexes' is owned by T.
-rw-r--r-- | libguile/threads.c | 27 |
1 files changed, 21 insertions, 6 deletions
diff --git a/libguile/threads.c b/libguile/threads.c index 457b1903f..8249ebf90 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -38,6 +38,8 @@ #include <sys/time.h> #endif +#include <assert.h> + #include "libguile/validate.h" #include "libguile/root.h" #include "libguile/eval.h" @@ -466,7 +468,12 @@ do_thread_exit (void *v) fat_mutex *m = SCM_MUTEX_DATA (mutex); scm_i_pthread_mutex_lock (&m->lock); + + /* Since MUTEX is in `t->mutexes', T must be its owner. */ + assert (scm_is_eq (m->owner, t->handle)); + unblock_from_queue (m->waiting); + scm_i_pthread_mutex_unlock (&m->lock); } @@ -1234,10 +1241,10 @@ fat_mutex_lock (SCM mutex, scm_t_timespec *timeout, SCM owner, int *ret) scm_i_pthread_mutex_lock (&t->admin_mutex); /* Only keep a weak reference to MUTEX so that it's not - retained when not referenced elsewhere (bug #27450). Note - that the weak pair itself it still retained, but it's better - than retaining MUTEX and the threads referred to by its - associated queue. */ + retained when not referenced elsewhere (bug #27450). + The weak pair itself is eventually removed when MUTEX + is unlocked. Note that `t->mutexes' lists mutexes + currently held by T, so it should be small. */ t->mutexes = scm_weak_car_pair (mutex, t->mutexes); scm_i_pthread_mutex_unlock (&t->admin_mutex); @@ -1409,7 +1416,11 @@ fat_mutex_unlock (SCM mutex, SCM cond, if (m->level > 0) m->level--; if (m->level == 0) - m->owner = unblock_from_queue (m->waiting); + { + /* Change the owner of MUTEX. */ + t->mutexes = scm_delq_x (mutex, t->mutexes); + m->owner = unblock_from_queue (m->waiting); + } t->block_asyncs++; @@ -1453,7 +1464,11 @@ fat_mutex_unlock (SCM mutex, SCM cond, if (m->level > 0) m->level--; if (m->level == 0) - m->owner = unblock_from_queue (m->waiting); + { + /* Change the owner of MUTEX. */ + t->mutexes = scm_delq_x (mutex, t->mutexes); + m->owner = unblock_from_queue (m->waiting); + } scm_i_pthread_mutex_unlock (&m->lock); ret = 1; |