[sheepdog] [PATCH] lib/logger: nonblocking waitpid to avoid sheep process cannot exit

[Ruoyu]

On 2014年06月11日 10:09, Hitoshi Mitake wrote:
> At Wed, 11 Jun 2014 11:05:53 +0900,
> Hitoshi Mitake wrote:
>> At Tue, 10 Jun 2014 18:22:11 +0800,
>> Ruoyu wrote:
>>> Sometimes sheep process cannot exit as we expected. I think the
>>> problem might be waitpid, the system call waiting for process to
>>> change state.
>>>
>>> Current log_close function calling waitpid as a void method. It is
>>> better to retrieve the return value and pass the nonblocking flag,
>>> that is WNOHANG, to it.
>>>
>>> Signed-off-by: Ruoyu <liangry at ucweb.com>
>>> ---
>>>   lib/logger.c | 24 ++++++++++++++++++------
>>>   1 file changed, 18 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/lib/logger.c b/lib/logger.c
>>> index 6829f45..79c60a7 100644
>>> --- a/lib/logger.c
>>> +++ b/lib/logger.c
>>> @@ -711,13 +711,25 @@ int log_init(const char *program_name, enum log_dst_type type, int level,
>>>   
>>>   void log_close(void)
>>>   {
>>> -	if (la) {
>>> -		la->active = false;
>>> -		waitpid(logger_pid, NULL, 0);
>>> +	if (!la)
>>> +		return;
>>>   
>>> -		syslog(LOG_WARNING, "logger pid %d stopped\n", logger_pid);
>>> -		closelog();
>>> -		free_logarea();
>>> +	while (true) {
>>> +		la->active = false;
> Seems that the above assignment statement can be placed before the loop.
>
> Thanks,
> Hitoshi
No, there is a sequential problem inter process if the statement is 
placed before the loop.

Suppose this situation:

Father process: -----> active = false 
---------------------------------------------------------------------------> 
waitpid
Child process: -------------------------------> active = true -----> 
while (active) do something ----->

The log process will loop forever so that main process cannot exit.
>>> +		pid_t pid = waitpid(logger_pid, NULL, WNOHANG);
>> Interleaved statement and declaration is not allowed in the current
>> coding style of sheepdog.
>> # Personally I really like it but it is denied in the past.
>>
>> Other part looks good and quite reasonable to me.
>>
>> Thanks,
>> Hitoshi
>>
>>> +		if (pid == 0) {
>>> +			usleep(100000);
>>> +			continue;
>>> +		} else if (pid > 0) {
>>> +			syslog(LOG_WARNING, "logger pid %d stopped\n",
>>> +					logger_pid);
>>> +			closelog();
>>> +			free_logarea();
>>> +			break;
>>> +		} else {
>>> +			syslog(LOG_ERR, "waitpid() failure\n");
>>> +			exit(1);
>>> +		}
>>>   	}
>>>   }
>>>   
>>> -- 
>>> 1.8.3.2
>>>
>>>
>>> -- 
>>> sheepdog mailing list
>>> sheepdog at lists.wpkg.org
>>> http://lists.wpkg.org/mailman/listinfo/sheepdog