[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