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

[Qiang Huang]

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.

But yes, I agree with the other patch you sent, maybe better solution,
I'll check that again.

> 
>> Signed-off-by: Qiang Huang <h.huangqiang at huawei.com>
> 
> I'm basically out this week and may not be thinking about it rightly,
> but I'm going to say
> 
> Nacked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> 
> in the hopes that someone with enough time will reconsider.
> 
>> ---
>>  src/lxc/lxccontainer.c | 8 +++-----
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
>> index 0bebdff..ddea0d7 100644
>> --- a/src/lxc/lxccontainer.c
>> +++ b/src/lxc/lxccontainer.c
>> @@ -632,12 +632,10 @@ reboot:
>>  		goto reboot;
>>  	}
>>
>> -	if (daemonize) {
>> +	if (daemonize)
>>  		lxc_container_put(c);
>> -		exit (ret == 0 ? true : false);
>> -	} else {
>> -		return (ret == 0 ? true : false);
>> -	}
>> +	
>> +	return (ret == 0 ? true : false);
>>  }
>>
>>  /*
>> -- 
>> 1.8.3
>>
> 
>