[SSSD] [PATCH] Add common SIGCHLD handling.

Jakub Hrozek jhrozek at redhat.com
Wed Dec 14 14:09:33 UTC 2011


On Tue, Dec 13, 2011 at 11:14:31AM -0500, Stephen Gallagher wrote:
> On Tue, 2011-11-22 at 08:58 -0500, Stephen Gallagher wrote:
> > On Wed, 2011-11-09 at 15:31 +0100, Pavel Zuna wrote:
> > > Updated patch attached.
> > 
> > 
> > 
> > Nack.
> > 
> > Please make "struct sss_sigchild_ctx" an opaque structure and provide an
> > init routine to create the hash and set up the signal handler. This way
> > we only have to call
> > sss_sigchild_init(ctx, &ctx->sigchld_ctx);
> > in be_process_init() (or any other executable that needs child control).
> > 
> > In sss_child_handler(), don't consider it fatal if the PID isn't found
> > in the hash. It could mean that something we're linked against did a
> > fork and was trying to maintain itself. This is bad, but not fatal.
> > Lower the debug level a bit (I think SSSDBG_OP_FAILURE is fine) and
> > allow the waitpid() loop to continue, rather than exiting immediately.
> > 
> > sss_child_ctx should be moved to child_common.c. Its internals should
> > not be visible elsewhere.
> > 
> > Please reorder sss_child_invoke_cb() to remove the entry from the hash
> > before invoking the callback (you cannot assume that the callback won't
> > do something funny to the hash structure or child_ctx, though it should
> > be safe). Also, please raise the debug level for this failure to
> > SSSDBG_OP_FAILURE.
> 
> I've taken over the development of this patch. I'm sending my changes to
> address the above issues except for a slight modification to my comments
> about the sss_child_handler loop - it was already ignoring unknown
> errors, but it was exiting on unexpected failures. I added a comment and
> will allow it to try to continue looping through in the hopes that it
> will minimize the potential zombies.
> 
> This patch does not currently consume the functions that it creates. I
> am working on converting the existing child handlers to use this code,
> but I want to get this patch looked at and acked in the meantime.

Are you going to fix the two comments you had originally in the later
patch? Those were don't consider it fatal if the PID isn't found
in the hash in sss_child_handler() and reordering sss_child_invoke_cb()

If so, then ack



More information about the sssd-devel mailing list