Comment 5 for bug 509647

Revision history for this message
Serge Hallyn (serge-hallyn) wrote : Re: [Bug 509647] Re: [MIR] lxc

Hi Seth, thanks very much for the review.

> Everything else:
>
> - Calling lxc_container_free() _after_ releasing the privlock feels wrong
> - lxc_container_get() may follow stale pointers while locking
> - lxc_container_put() could be freeing an object that was acquired

I think this was an unintented bug. Note that

 1. lxc_container_free() sets c->privlock to NULL
 2. lxcunlock() returns an error -2 if c->privlock is NULL
 3. lxclock() doesn't do so - I probably accidentally dropped
    or forgot to add in that check.

since lxc_container_get() and _put() both return false if lxclock
fails, I adding the needed check in lxclock() this should suffice.

Does that sound ok to you for this item?

If you're up for it, I also wouldn't mind chatting sometime about
the goals and design of the locking. (The comment at top of
lxccontainer.c pretty accurately sums up my goals. It doesn't
mention that running containers are already mutexed by the
monitor UNIX socket in $lxc_path.)

-serge