On Mon, 2011-12-19 at 12:59 +0100, Jakub Hrozek wrote:
On Wed, Dec 14, 2011 at 10:00:58PM -0500, Stephen Gallagher wrote:
> On Wed, 2011-12-14 at 13:03 -0500, Stephen Gallagher wrote:
> > On Wed, 2011-12-14 at 09:15 -0500, Stephen Gallagher wrote:
> > >
> > > On Dec 14, 2011, at 9:09 AM, Jakub Hrozek <jhrozek(a)redhat.com>
wrote:
> > > >
> > > > 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.
> >
> >
> > New patch attached fixes the missing reordering of the
> > sssd_child_invoke_cb() and adds more explicit handling of unknown PIDs
> > (with a new DEBUG log message).
>
> I discovered two more issues with this patch, one serious, one cosmetic
> but annoying.
>
> 1) sss_child_register() wasn't assigning child->sigchld_ctx, so if the
> child was freed, the destructor would segfault trying to remove this
> child from the hash table.
>
> 2) sss_child_handler() needed to ignore a return of pid=0 (which would
> happen every time as the last pass through) or else it would print an
> error trying to find pid 0 in the hash table (which it had better not
> be. I don't want SSSD monitoring init!)
init is pid=1 :-)
>
> New patches attached.
Ack to this version.
Pushed to master.