1
Fork 0
mirror of https://git.savannah.gnu.org/git/guile.git synced 2025-05-20 11:40:18 +02:00

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.
This commit is contained in:
Ludovic Courtès 2010-09-02 14:42:14 +02:00
parent d31ae2c363
commit f57fdf07d6

View file

@ -38,6 +38,8 @@
#include <sys/time.h> #include <sys/time.h>
#endif #endif
#include <assert.h>
#include "libguile/validate.h" #include "libguile/validate.h"
#include "libguile/root.h" #include "libguile/root.h"
#include "libguile/eval.h" #include "libguile/eval.h"
@ -466,7 +468,12 @@ do_thread_exit (void *v)
fat_mutex *m = SCM_MUTEX_DATA (mutex); fat_mutex *m = SCM_MUTEX_DATA (mutex);
scm_i_pthread_mutex_lock (&m->lock); 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); unblock_from_queue (m->waiting);
scm_i_pthread_mutex_unlock (&m->lock); 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); scm_i_pthread_mutex_lock (&t->admin_mutex);
/* Only keep a weak reference to MUTEX so that it's not /* Only keep a weak reference to MUTEX so that it's not
retained when not referenced elsewhere (bug #27450). Note retained when not referenced elsewhere (bug #27450).
that the weak pair itself it still retained, but it's better The weak pair itself is eventually removed when MUTEX
than retaining MUTEX and the threads referred to by its is unlocked. Note that `t->mutexes' lists mutexes
associated queue. */ currently held by T, so it should be small. */
t->mutexes = scm_weak_car_pair (mutex, t->mutexes); t->mutexes = scm_weak_car_pair (mutex, t->mutexes);
scm_i_pthread_mutex_unlock (&t->admin_mutex); scm_i_pthread_mutex_unlock (&t->admin_mutex);
@ -1409,7 +1416,11 @@ fat_mutex_unlock (SCM mutex, SCM cond,
if (m->level > 0) if (m->level > 0)
m->level--; m->level--;
if (m->level == 0) 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++; t->block_asyncs++;
@ -1453,7 +1464,11 @@ fat_mutex_unlock (SCM mutex, SCM cond,
if (m->level > 0) if (m->level > 0)
m->level--; m->level--;
if (m->level == 0) 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); scm_i_pthread_mutex_unlock (&m->lock);
ret = 1; ret = 1;