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.