At Wed, 11 Jun 2014 10:30:01 +0800,
Ruoyu wrote:
> 
> 
> 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 <[email protected]>
> >>> ---
> >>>   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.

Ah, sorry, I missed the case. You are correct.

Thanks,
Hitoshi

> >>> +         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
> >>> [email protected]
> >>> http://lists.wpkg.org/mailman/listinfo/sheepdog
> 
> 
-- 
sheepdog mailing list
[email protected]
http://lists.wpkg.org/mailman/listinfo/sheepdog

Reply via email to