Re: Bug in PoCo::Child when PID wraps round

[Ricky Egeland]

Hi Erick,




Cheers,
Ricky


=== BEGIN PATCH ===
*** Child.pm    2009-06-02 16:10:53.120660674 +0200
--- Child.pm.patched    2009-05-28 16:38:08.687685140 +0200
***************
*** 265,279 ****

        # clean up

!       delete $self->{$PKG}{wheels}{$id};
!       delete $self->{$PKG}{pids}{$id};

        # all expiring children should issue a "done" except when
        # the return code is non-zero which indicates a failure
        # if the caller asked we quit, fire a "done" regardless
        # of the child's return code value (we might have hard killed)

!       my $event = ($self->{$PKG}{wheels}{$id}{quit} || $rc == 0)
                ? "done" : "died"
                ;
        $self->callback($event, { wheel => $id, rc => $rc });
--- 265,281 ----

        # clean up

!       delete $self->{$PKG}{pids}{$self->{$PKG}{wheels}{$id}{ref}->PID};
!         my $wheel = delete $self->{$PKG}{wheels}{$id};
!       delete $self->{$PKG}{SIGCHLD}{$id};
!       delete $self->{$PKG}{CLOSED}{$id};

        # all expiring children should issue a "done" except when
        # the return code is non-zero which indicates a failure
        # if the caller asked we quit, fire a "done" regardless
        # of the child's return code value (we might have hard killed)

!       my $event = ($wheel->{quit} || $rc == 0)
                ? "done" : "died"
                ;
        $self->callback($event, { wheel => $id, rc => $rc });
***************
*** 306,311 ****
--- 308,314 ----
  sub wheel {
        my $self = shift;
        my $id = shift || $self->{$PKG}{wheels}{current};
+       return undef unless exists $self->{$PKG}{wheels}{$id};
        $self->{$PKG}{wheels}{$id}{ref};
        }
=== END PATCH ===


On Jun 4, 2009, at 7:30 PM, Erick Calder wrote:

I work with Tony. I have a new patch now which addresses this issue, as well as another leak: calling $self->wheel($id) will initialize a key in $self->{$PKG}{wheels}{$id}, causing a leak if the wheel had already finished or if it never existed.

sorry, I still own it. I've just been disconnected from the world a long

time.  it will take me a while to get back to you on this but I will

From: Tony Wildish <wild...@mail.cern.ch>
Date: Thu, 7 May 2009 10:58:12 +0200
To: <poe@perl.org>
Subject: Re: Bug in PoCo::Child when PID wraps round (fwd)

Hi,


it orphaned?

 TIA,
 Tony.

On Wed, 15 Apr 2009, Tony Wildish wrote:

just checking one more time: is anyone looking after PoCo::Child, or is

Hi,


but it looks like that list is dead. Is there someone here managing
PoCo::Child?

 Cheers,
 Tony.

---------- Forwarded message ----------
Date: Tue, 24 Mar 2009 12:56:33 +0100 (CET)
From: Tony Wildish <wild...@mail.cern.ch>
To: bug-poe-component-ch...@rt.cpan.org
Subject: Bug in PoCo::Child when PID wraps round

Hi,


wrong wheelID will be given to the 'done' event.

 I'm using PoCo::Child version 1.39, perl 5.8.5, kernel
2.6.9-78.0.8.EL.cernsmp.

 You can see the bug by inspection of the code. At line 192 we have:

        my $id = $wheel->ID;


        $self->{$PKG}{pids}{$wheel->PID} = $id;


clean up deletes as follows:

        delete $self->{$PKG}{wheels}{$id};
        delete $self->{$PKG}{pids}{$id};

 so it's deleting keyed on wheel->ID, not $wheel->PID.



sub sig_child {
    my ($kernel, $self, $pid, $rc) = @_[KERNEL, OBJECT, ARG1, ARG2];

        my $id = $self->{$PKG}{pids}{$pid} || "";

    # child death signals are issued by the OS and sent to all
    # sessions; we want to handle only our own children

        return unless $id;


will not return here, and will cause 'done' to be fired with the old
wheelID, which is plainly wrong.

 Also, the cleanup around line 268/269 should remove {CLOSED} and

but it's still a leak.

 Cheers,
 Tony.

I sent this to the bug-poe-component-ch...@rt.cpan.org list some time ago we have been using PoCo::Child in a project for a while now and have just uncovered a bug. The internal bookkeeping for the association of wheelIDs and PIDs is wrong, with the result that when the OS PID wraps round, the $self->debug(qq/run(): "@$cmd", wheel=$id, pid=/ . $wheel- >PID); so the {pids} are inserted keyed on $wheel->PID. But at line 268, the When the PIDs wrap round, the protection in sig_child is broken by this: Because the {pids} hash had the wrong entry deleted in the cleanup, when the PIDs wrap, an old $id will be found where none should be. So sig_child {SIGCHLD} entries for the wheel too. That's less serious because it won't cause a problem until wheel-IDs wrap round, which will take a lot longer,

smime.p7s
Description: S/MIME cryptographic signature