1. in container_free, set c->privlock to NULL before calling sem_destroy, to prevent a window where another thread could call sem_wait(c->privlock) while c->privlock is not NULL but is already destroyed.
2. in container_get, check for numthreads < 0 before calling lxclock. Once numthreads is 0, it never goes back up. Following is a comment added to lxccontainer.c: /* * Consider the following case: freer | racing get()er ================================================================== lxc_container_put() | lxc_container_get() \ lxclock(c->privlock) | c->numthreads < 1? (no) \ c->numthreads = 0 | \ lxclock(c->privlock) -> waits \ lxcunlock() | \ \ lxc_container_free() | \ lxclock() returns | \ c->numthreads < 1 -> return 0 \ \ (free stuff) | \ \ sem_destroy(privlock) | * When the get()er checks numthreads the first time, one of the following * is true: * 1. freer has set numthreads = 0. get() returns 0 * 2. freer is between lxclock and setting numthreads to 0. get()er will * sem_wait on privlock, get lxclock after freer() drops it, then see * numthreads is 0 and exit without touching lxclock again.. * 3. freer has not yet locked privlock. If get()er runs first, then put()er * will see --numthreads = 1 and not call lxc_container_free(). */ Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com> --- src/lxc/lxccontainer.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index a4376b4..e09b3fb 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -72,9 +72,10 @@ static void lxc_container_free(struct lxc_container *c) c->slock = NULL; } if (c->privlock) { - sem_destroy(c->privlock); - free(c->privlock); + sem_t *l = c->privlock; c->privlock = NULL; + sem_destroy(l); + free(l); } if (c->name) { free(c->name); @@ -91,11 +92,39 @@ static void lxc_container_free(struct lxc_container *c) free(c); } +/* + * Consider the following case: +freer | racing get()er +================================================================== +lxc_container_put() | lxc_container_get() +\ lxclock(c->privlock) | c->numthreads < 1? (no) +\ c->numthreads = 0 | \ lxclock(c->privlock) -> waits +\ lxcunlock() | \ +\ lxc_container_free() | \ lxclock() returns + | \ c->numthreads < 1 -> return 0 +\ \ (free stuff) | +\ \ sem_destroy(privlock) | + + * When the get()er checks numthreads the first time, one of the following + * is true: + * 1. freer has set numthreads = 0. get() returns 0 + * 2. freer is between lxclock and setting numthreads to 0. get()er will + * sem_wait on privlock, get lxclock after freer() drops it, then see + * numthreads is 0 and exit without touching lxclock again.. + * 3. freer has not yet locked privlock. If get()er runs first, then put()er + * will see --numthreads = 1 and not call lxc_container_free(). +*/ + int lxc_container_get(struct lxc_container *c) { if (!c) return 0; + // if someone else has already started freeing the container, don't + // try to take the lock, which may be invalid + if (c->numthreads < 1) + return 0; + if (lxclock(c->privlock, 0)) return 0; if (c->numthreads < 1) { -- 1.8.1.2 ------------------------------------------------------------------------------ Precog is a next-generation analytics platform capable of advanced analytics on semi-structured data. The platform includes APIs for building apps and a phenomenal toolset for data science. Developers can use our toolset for easy data analysis & visualization. Get a free account! http://www2.precog.com/precogplatform/slashdotnewsletter _______________________________________________ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel