[SSSD] [PATCH] Add common SIGCHLD handling.

Stephen Gallagher sgallagh at redhat.com
Wed Dec 14 14:15:03 UTC 2011



On Dec 14, 2011, at 9:09 AM, Jakub Hrozek <jhrozek at redhat.com> wrote:

> 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


The first part of that I already described above as being an incorrect reading of the source. I missed the second part and I'll fix that shortly.

> _______________________________________________
> 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