Skip to content

Commit

Permalink
Be more careful about the order in which we kill processes.
Browse files Browse the repository at this point in the history
In particular, don't have the watchdog kill itself. Even though
perl's `kill` takes a list of pids, they actually get signalled
one at a time, and so if the watchdog is in the middle of the
list not everything will get killed like it should be. So have
the watchdog filter itself out of the list, and then just
naturally die.
  • Loading branch information
DrHyde committed Apr 14, 2023
1 parent 3b85f1b commit 90cf7a2
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 8 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
1.0.2 2023-04-14

- Be more careful about the order in which we kill processes
for greater reliability. If the watchdog killed itself too
early it might miss other child processes.

1.0.1 2021-12-18

- Make tests more reliable under heavy load
Expand Down
8 changes: 4 additions & 4 deletions Makefile.PL
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ WriteMakefile(
bin/ptimeout
)],
PREREQ_PM => {
Capture::Tiny => 0,
Proc::Killfam => '1.0',
Test::Differences => 0,
Test::More => '0.88', # done_testing
Capture::Tiny => 0,
Proc::ProcessTable => '0.634',
Test::Differences => 0,
Test::More => '0.88', # done_testing
},
);
30 changes: 27 additions & 3 deletions lib/App/ptimeout.pm
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use strict;
use warnings;
no warnings 'numeric';

use Proc::Killfam;
use Proc::ProcessTable;

our $VERSION = '1.0.1';
our $VERSION = '1.0.2';

sub _run {
my($timeout, @argv) = @_;
Expand All @@ -27,10 +27,34 @@ sub _run {
} else { # watchdog child process
sleep $timeout;
warn "timed out\n";
killfam SIGTERM => getppid;
# We can't just `Proc::Killfam::killfam TERM => getppid` because that
# will put the watchdog process somewhere in the list of victims, and
# thus anything after it in the list won't get killed. Filter this
# process out of the list of victims and kill everything else. The
# watchdog will then exit normally.
my $process_table = Proc::ProcessTable->new->table;
my @victims = grep { $_ != $$ } _get_pids($process_table, getppid);
kill SIGTERM => @victims, getppid;
}
}

# Copied from Proc::Killfam::get_pids in Proc-ProcessTable-0.634 which is
# GPL/Artistic licenced. It's undocumented there so should be considered
# unstable, hence why copied.
sub _get_pids {
my($procs, @kids) = @_;
my @pids;
foreach my $kid (@kids) {
foreach my $proc (@$procs) {
if ($proc->ppid == $kid) {
my $pid = $proc->pid;
push @pids, $pid, _get_pids($procs, $pid);
}
}
}
@pids;
}

=head1 NAME
App::ptimeout - module implementing L<ptimeout>
Expand Down
6 changes: 5 additions & 1 deletion t/ptimeout.t
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ eq_or_diff(
);

($stdout, $stderr, $status) = capture(sub {
system(
my $status = system(
@perl,
1,
q{
Expand All @@ -43,6 +43,10 @@ eq_or_diff(
"
}
) >> 8;
# keep capturing output until the sub-process would have finished
# were it not killed
sleep 15;
$status;
});
eq_or_diff(
[$status, $stderr, $stdout],
Expand Down

0 comments on commit 90cf7a2

Please sign in to comment.