Hi,

 I sent this to the bug-poe-component-ch...@rt.cpan.org list some time ago 
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,

 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 
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->debug(qq/run(): "@$cmd", wheel=$id, pid=/ . $wheel->PID);

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

 so the {pids} are inserted keyed on $wheel->PID. But at line 268, the 
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.

 When the PIDs wrap round, the protection in sig_child is broken by this:

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;

 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
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
{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,
but it's still a leak.

 Cheers,
 Tony.

Reply via email to