Skip to content

Commit ba7bb38

Browse files
committed
Fix handling process groups when initial process is not running anymore
One can set the `kill_whole_group` attribute to make the `stop` function of `ReadWriteProcess` stop the whole process group. This works in general but there are a few flaws this change addresses: * Make `is_running` consider the whole process group as well. Otherwise, when the initial process is not running anymore `stop` returns early, even though there are still other processes left in the group. * Avoid the use of `getpgrp` in the `stop` function. This function needs the PID to be still valid but this might not be the case when the initial process is not running anymore. The group ID is simply the PID of the initial process so we can just use that. (Assuming the the process group was created by invoking `setpgrp(0, 0)` in the code callback.) I tested this by creating a simple script that will leave a leftover process¹. In the code callback I executed `setpgrp(0, 0)` followed by `exec(…)` on that script. Without this change the a process is left behind when calling `stop` (see `ps -aux | grep 'sleep infinity'`). With this change the process is correctly cleaned up. I used existing code of openQA (`isotovideo.pm`) to test this. Related issue: https://progress.opensuse.org/issues/170209 --- ¹ ``` #!/bin/bash sleep infinity & ```
1 parent f04eb4e commit ba7bb38

1 file changed

Lines changed: 5 additions & 3 deletions

File tree

lib/Mojo/IOLoop/ReadWriteProcess.pm

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,8 @@ sub restart {
403403
sub is_running {
404404
my ($self) = shift;
405405
$self->session->consume_collected_info;
406-
$self->process_id ? kill 0 => $self->process_id : 0;
406+
return 0 unless my $pid = $self->process_id;
407+
kill(0, ($self->kill_whole_group ? -$pid : $pid));
407408
}
408409

409410
sub write_pidfile {
@@ -519,15 +520,16 @@ sub stop {
519520
my $pid = $self->pid;
520521
return $self unless defined $pid;
521522
return $self->_shutdown(1) unless $self->is_running;
522-
$self->_diag("Stopping $pid") if DEBUG;
523523

524524
my $ret;
525525
my $attempt = 1;
526526
my $timeout = $self->total_sleeptime_during_kill // 0;
527527
my $sleep_time = $self->sleeptime_during_kill;
528528
my $max_attempts = $self->max_kill_attempts;
529529
my $signal = $self->_default_kill_signal;
530-
$pid = -getpgrp($pid) if $self->kill_whole_group;
530+
$pid = -$pid if $self->kill_whole_group;
531+
$self->_diag("Stopping $pid") if DEBUG;
532+
531533
until ((defined $ret && ($ret == $pid || $ret == -1))
532534
|| ($attempt > $max_attempts && $timeout <= 0))
533535
{

0 commit comments

Comments
 (0)