[SSSD] [PATCH] Wait for all children to exit

Sumit Bose sbose at redhat.com
Wed Nov 17 09:15:25 UTC 2010


On Thu, Nov 11, 2010 at 09:05:57AM -0500, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Previously, there was a race-condition where the monitor might
> terminate before its children.
> 
> Resolves https://fedorahosted.org/sssd/ticket/555

Hi,

please find my comments below. Do you have a good reproducer for this
issue ?

bye,
Sumit

> 
> - -- 
> Stephen Gallagher
> RHCE 804006346421761
> 
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAkzb+EUACgkQeiVVYja6o6NbAgCeJRn2o4WHjGKkQ8w/ezThS3lN
> R9AAnA3yTkPBCgkK069cG0n5XtzR9Oqw
> =DQHE
> -----END PGP SIGNATURE-----

> From bbf8b3bdc806870f7067244c3f5a1b5eb37de21f Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Thu, 11 Nov 2010 09:04:22 -0500
> Subject: [PATCH] Wait for all children to exit
> 
> Previously, there was a race-condition where the monitor might
> terminate before its children.
> ---
>  src/monitor/monitor.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 60 insertions(+), 2 deletions(-)
> 
> diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
> index 6479f7a9fd5877e7b5baaaee4f3f92001506d730..a2bf4fadc28eaad9acd82c590da42dcd601da674 100644
> --- a/src/monitor/monitor.c
> +++ b/src/monitor/monitor.c
> @@ -1171,16 +1171,74 @@ static void monitor_quit(struct tevent_context *ev,
>                           void *siginfo,
>                           void *private_data)
>  {
> +    struct mt_ctx *mt_ctx = talloc_get_type(private_data, struct mt_ctx);
> +    struct mt_svc *svc;
> +    pid_t pid;
> +    int status;
> +    errno_t error;
> +
>      DEBUG(8, ("Received shutdown command\n"));
> -    monitor_cleanup();
> +
> +    DEBUG(0, ("Monitor received %s: terminating children\n",
> +              strsignal(signum)));
> +
> +    /* Kill all of our known children manually */
> +    DLIST_FOR_EACH(svc, mt_ctx->svc_list) {
> +        if (svc->pid == 0) {
> +            /* The local provider has no PID */
> +            continue;
> +        }
> +
> +        DEBUG(1, ("Terminating [%s]\n", svc->name));
> +        kill(svc->pid, SIGTERM);

I would suggest to first send SIGTERM to all children and then wait for
them to finish.

> +
> +        do {
> +            errno = 0;
> +            pid = waitpid(svc->pid, &status, 0);

If the child is in a bad state we might wait here forever. Maybe using
WNOHANG and some timeout before sending a SIGKILL would be more
reliable?


> +            if (pid == -1) {
> +                /* An error occurred while waiting */
> +                error = errno;
> +                if (error != EINTR) {
> +                    DEBUG(0, ("[%d][%s] while waiting for [%s]\n",
> +                              error, strerror(error), svc->name));
> +                    /* Forcibly kill this child */
> +                    kill(svc->pid, SIGKILL);
> +                    break;
> +                }
> +            } else {
> +                error = 0;
> +                if WIFEXITED(status) {
> +                    DEBUG(1, ("Child [%s] exited gracefully\n", svc->name));
> +                } else if WIFSIGNALED(status) {
> +                    DEBUG(1, ("Child [%s] terminated with a signal\n", svc->name));
> +                } else {
> +                    DEBUG(0, ("Child [%s] did not exit cleanly\n", svc->name));
> +                    /* Forcibly kill this child */
> +                    kill(svc->pid, SIGKILL);
> +                }
> +            }
> +        } while (error == EINTR);
> +    }
>  
>  #if HAVE_GETPGRP
> +    /* Kill any remaining children in our process group, just in case
> +     * we have any leftover children we don't expect. For example, if
> +     * a krb5_child or ldap_child is running at the same moment.
> +     */
>      if (getpgrp() == getpid()) {
> -        DEBUG(0,("%s: killing children\n", strsignal(signum)));
>          kill(-getpgrp(), SIGTERM);
> +        do {
> +            errno = 0;
> +            pid = waitpid(0, &status, 0);

WNOHANG ?

> +            if (pid == -1) {
> +                error = errno;
> +            }
> +        } while (error == EINTR || pid > 0);
>      }
>  #endif
>  
> +    monitor_cleanup();
> +
>      exit(0);
>  }
>  
> -- 
> 1.7.3.2
> 


> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list