[PATCH] start-stop-daemon fails to stop processes and sometimes fails to start them.

[Joakim Tjernlund]

> -----Original Message-----
> From: Denys Vlasenko [mailto:vda.linux at googlemail.com]
> Sent: den 19 april 2008 22:18
> To: Joakim Tjernlund
> Cc: busybox at busybox.net
> Subject: Re: [PATCH] start-stop-daemon fails to stop processes and sometimes fails to start them.
> 
> On Saturday 19 April 2008 21:34, Joakim Tjernlund wrote:
> > > I think that you saw something in real world usage and you had to make
> > > this change in order to make it work reliably.
> >
> > very true, it is not working at all when you try doing lots of
> > start-stop-demon in a row.
> >
> > >
> > > But please realize that your change, as written, will not tell casual
> > > code reader WHY it is required, and it bears the risk of this code
> > > being *deleted again* under impression that it is not needed.
> > >
> > > In order to prevent that, you have to give at least one real example
> > > where it is needed.
> >
> > Well aware of this. Been doing lots of uClibc development in the past.
> >
> > hmm, I think I did that with my latest round of patches.
> 
> I see.
> 
> --- busybox.8/debianutils/start_stop_daemon.c   2008-04-19 21:01:58.000000000 +0200
> +++ busybox.9/debianutils/start_stop_daemon.c   2008-04-19 22:12:26.000000000 +0200
> @@ -145,11 +145,17 @@ static void do_procinit(void)
>         procdir = xopendir("/proc");
> 
>         pid = 0;
> -       while ((entry = readdir(procdir)) != NULL) {
> -               pid = bb_strtou(entry->d_name, NULL, 10);
> -               if (errno)
> -                       continue;
> -               check(pid);
> +       while(1) {
> +               errno = 0; /* clear any previous error */
> +               entry = readdir(procdir);
> +               if (errno) /* Stale entry, process has died after opendir */
> +                       continue;
> +               if (!entry) /* EOF, no more entries */
> +                       break;
> +               pid = bb_strtou(entry->d_name, NULL, 10);
> +               if (errno) /* NaN */
> +                       continue;
> +               check(pid);
>         }
>         closedir(procdir);
>         if (!pid)
> 
> 
> My concern is that you loop back to readdir on ANY error.
> I was bitten by this kind of code myself.

I see, why didn't you say so earlier. Have you seen this
on /proc too?

> 
> The problem is that some errors are fatal: if you redo
> the operation, it will simply fail again with same error.
> 
> The best thing to do is to test for specific errno values
> which you *know* are not of this kind and will not lead
> to infinite looping.
> 
> Do you remember what errno was returned when you saw
> this bug in action?

No, didn't check as readdir only have one possibly errno
listed in the man page: EBADF
There isn't any other error to test for. You can change
it if you like to test for that errno if you think it is
relevant.

   Jocke
> --
> vda