[lxc-devel] [PATCH 1/2] lxc-start: fix the container leak when daemonize

[Qiang Huang]

On 2014/1/21 11:22, Serge Hallyn wrote:
> Quoting Qiang Huang (h.huangqiang at huawei.com):
>> Hi Serge,
>>
>> On 2014/1/20 23:44, Serge Hallyn wrote:
>>> Quoting Qiang Huang (h.huangqiang at huawei.com):
>>>> On 2014/1/20 1:26, Serge Hallyn wrote:
>>>>> Quoting Qiang Huang (h.huangqiang at huawei.com):
>>>>>> When start container with daemon model, we'll have a new daemon
>>>>>> process in lxcapi_start, whose c->numthreads is 2, inherited
>>>>>> from his father. Even his father return to main(), the
>>>>>> lxc_container_put won't affect son's numthreads.
>>>>>
>>>>> The memlock is only between threads.  But the child is fork()ed, so the
>>>>> lxc_container_put() of one won't affect the other task.
>>>>>
>>>>> Now maybe wait_on_daemonized_start() should be doing an
>>>>> lxc_container_put()?
>>>>>
>>>>>> So when daemon stops, he should return to main and do
>>>>>> lxc_container_put again, rather than exit and leave the
>>>>>> container alone.
>>>>>
>>>>> I disagree.  This means two separate processes will continue at the
>>>>> point where lxcapi_start() was called.  That sounds like a recipe for
>>>>> disaster.
>>>>
>>>> Well, I thought as long as child haven't exec(), he'll still have
>>>> the same context as his father, where lxcapi_start() was called
>>>> is part of it. So I don't see how disaster that will be.
>>>
>>> The disaster would be that the caller might do something like:
>>>
>>> 	c->want_daemonize(true);
>>> 	if (c->start()) {
>>> 		FILE *f = fopen("/run/running_containers", "a");
>>> 		bump_container_count(f);
>>> 		fclose(f);
>>> 	}
>>>
>>> Now the caller's thread will bump the running_containers count,
>>> and then after the container shuts down, the (daemonized)
>>> monitor thread (which never does an exec) will return and also
>>> bump that count, making the count in /run/running_containers
>>> wrong.
>>
>> The real world daemonized monitor thread dose an exec ;)
> 
> When lxcapi_start() forks, the new task in turn will also clone,
> so that there is both a container monitor task and the actual
> init task.  The init task is the one that execs.  The monitor
> task does not do an exec.  That is the one which must exit when
> it is done.

Yeah, I understand this, I think we were talking about different
monitors, the one I said is from lxc_monitord_spawn, it exec()
to run lxc-monitord.

Anyway, I think we have got consensus view, I'll send new patches
with that PID file one.

Thanks again.

> 
> -serge
> 
> .
>