fix(node_info): reap zombie pve_mod_worker and orphaned collector processes

Double-forks pve_mod_worker so it is reparented to init instead of the calling process, which reaps it immediately on exit and prevents it lingering as a <defunct> zombie. Also adds startup-time reaping of orphaned collector-* processes (ppid==1) left behind if a worker was previously killed with SIGKILL, and an opportunistic waitpid(WNOHANG) in _worker_lock_file_exists as a cheap supplemental reap.
This commit is contained in:
Meliox 2026-08-18 20:39:26 +02:00
parent 69e2d78b0a
commit 4f5bc24104
2 changed files with 153 additions and 25 deletions

View File

@ -4,7 +4,7 @@ use strict;
use warnings;
use Exporter 'import';
use POSIX qw(WNOHANG);
use POSIX qw(WNOHANG setsid);
use File::Path qw(remove_tree);
use PVE::PVEMod::Config qw(
@ -13,7 +13,7 @@ use PVE::PVEMod::Config qw(
$pve_mod_worker_lock $startup_lock
);
use PVE::PVEMod::Utils qw(
debug is_process_alive read_lock_pid
debug is_process_alive get_process_ppid read_lock_pid
acquire_exclusive_lock ensure_pve_mod_directory_exists
check_executable startup_message
);
@ -113,16 +113,36 @@ sub notify_pve_mod_worker {
sub _worker_lock_file_exists {
return 0 unless -f $pve_mod_worker_lock;
my $pid = read_lock_pid($pve_mod_worker_lock);
if (defined $pid && $pid =~ /^(\d+)$/ && is_process_alive($1)) {
if (!defined $pid || $pid !~ /^(\d+)$/) {
debug(__LINE__, "Worker lock is invalid (PID: " . ($pid // 'undefined') . "), removing");
unlink($pve_mod_worker_lock);
return 0;
}
my $worker_pid = $1;
if (is_process_alive($worker_pid)) {
return 1;
}
debug(__LINE__, "Stale or zombie worker lock found, removing it");
debug(__LINE__, "Stale or zombie worker lock found for PID $worker_pid, reaping if needed");
my $reaped_pid = waitpid($worker_pid, WNOHANG);
if ($reaped_pid == $worker_pid) {
debug(__LINE__, "Reaped stale worker PID $worker_pid");
} elsif ($reaped_pid == -1 && $!{ECHILD}) {
# Expected once the worker is double-forked: we're not its parent, init is.
debug(__LINE__, "PID $worker_pid is not a child of this process; treating lock as stale");
} else {
debug(__LINE__, "waitpid on PID $worker_pid returned $reaped_pid: $!");
}
unlink($pve_mod_worker_lock);
return 0;
}
# Forks the worker process and records its PID in the lock file.
# Double-forks so the worker is reparented to init (PID 1) instead of the
# caller; init reaps it on exit, so it never lingers as a <defunct> zombie.
sub _pve_mod_worker {
debug(__LINE__, "_pve_mod_worker called");
@ -132,34 +152,57 @@ sub _pve_mod_worker {
print $pve_mod_worker_fh "$$\n";
close($pve_mod_worker_fh);
debug(__LINE__, "Forking new pve_mod_worker process");
my $pve_mod_worker_pid = fork();
debug(__LINE__, "Forking intermediate process for pve_mod_worker");
my $intermediate_pid = fork();
unless (defined $pve_mod_worker_pid) {
debug(__LINE__, "Failed to fork pve_mod_worker process: $!");
unless (defined $intermediate_pid) {
debug(__LINE__, "Failed to fork intermediate pve_mod_worker process: $!");
unlink($pve_mod_worker_lock);
return;
}
if ($pve_mod_worker_pid == 0) {
# Child
$0 = "pve_mod_worker_controller";
debug(__LINE__, "Child process forked, calling _pve_mod_keep_alive");
_pve_mod_keep_alive();
exit(0);
if ($intermediate_pid == 0) {
# Intermediate child: detach into its own session, fork the real
# worker, then exit immediately so the worker is reparented to init.
setsid();
my $worker_pid = fork();
unless (defined $worker_pid) {
debug(__LINE__, "Failed to fork pve_mod_worker process: $!");
unlink($pve_mod_worker_lock);
POSIX::_exit(1);
}
if ($worker_pid == 0) {
# Grandchild — the actual worker
$0 = "pve_mod_worker_controller";
if (open my $fh, '>', $pve_mod_worker_lock) {
print $fh "$$\n";
close $fh;
debug(__LINE__, "Wrote pve_mod_worker PID to lock file: $pve_mod_worker_lock");
} else {
debug(__LINE__, "Failed to write pve_mod_worker lock file: $!");
}
debug(__LINE__, "Worker process forked, calling _pve_mod_keep_alive");
_pve_mod_keep_alive();
exit(0);
}
debug(__LINE__, "Intermediate process exiting, worker PID $worker_pid reparented to init");
# _exit (not exit) so we skip END blocks/global destruction — this
# process only ever existed to perform the double fork.
POSIX::_exit(0);
}
# Parent — update lock file with real child PID
debug(__LINE__, "Forked pve_mod_worker process with PID $pve_mod_worker_pid");
if (open my $fh, '>', $pve_mod_worker_lock) {
print $fh "$pve_mod_worker_pid\n";
close $fh;
debug(__LINE__, "Wrote pve_mod_worker PID to lock file: $pve_mod_worker_lock");
# Parent — reap the short-lived intermediate process immediately, so it
# never has a chance to become a zombie under us either.
waitpid($intermediate_pid, 0);
my $status = $? >> 8;
if ($status == 0) {
debug(__LINE__, "pve_mod_worker process started successfully");
} else {
debug(__LINE__, "Failed to write pve_mod_worker lock file: $!");
kill('TERM', $pve_mod_worker_pid);
debug(__LINE__, "Intermediate pve_mod_worker process exited with status $status");
}
debug(__LINE__, "pve_mod_worker process started successfully");
}
# ============================================================================
@ -206,6 +249,8 @@ sub _pve_mod_keep_alive {
exit(0);
};
_reap_orphaned_collectors();
debug(__LINE__, "Worker starting all collectors");
_initialise_sensors_collector();
_initialise_graphics_collectors();
@ -458,6 +503,70 @@ sub _stop_child_collectors {
debug(__LINE__, "Cleanup complete");
}
# Finds and terminates collector processes left behind by a previous worker
# that never got to run _stop_child_collectors() (e.g. killed with SIGKILL).
# Only targets processes reparented to init, never a running worker's own.
sub _reap_orphaned_collectors {
debug(__LINE__, "Scanning for orphaned collector processes from a previous worker");
unless (opendir(my $dh, '/proc')) {
debug(__LINE__, "Failed to open /proc: $!");
return;
}
my @orphans;
while (my $entry = readdir($dh)) {
next unless $entry =~ /^(\d+)$/;
my $pid = $1;
next if $pid == $$;
next unless open(my $fh, '<', "/proc/$pid/cmdline");
my $cmdline = <$fh>;
close($fh);
next unless defined $cmdline;
my ($name) = split /\0/, $cmdline;
next unless defined $name && $name =~ /^collector-/;
my $ppid = get_process_ppid($pid);
next unless defined $ppid && $ppid == 1;
push @orphans, $pid;
}
closedir($dh);
unless (@orphans) {
debug(__LINE__, "No orphaned collectors found");
return;
}
debug(__LINE__, "Found " . scalar(@orphans) . " orphaned collector(s), terminating");
foreach my $pid (@orphans) {
kill('TERM', $pid);
debug(__LINE__, "Sent SIGTERM to orphaned collector PID $pid");
}
my $timeout = 2;
my $start = time();
while (time() - $start < $timeout) {
my $any_alive = 0;
foreach my $pid (@orphans) {
if (kill(0, $pid)) { $any_alive = 1; last; }
}
last unless $any_alive;
select(undef, undef, undef, 0.1);
}
foreach my $pid (@orphans) {
if (kill(0, $pid)) {
debug(__LINE__, "Force killing orphaned collector process $pid");
kill('KILL', $pid);
}
}
debug(__LINE__, "Orphan cleanup complete");
}
# ============================================================================
# END block — only the worker process performs cleanup
# ============================================================================

View File

@ -15,6 +15,7 @@ our @EXPORT_OK = qw(
debug
read_sysfs
is_process_alive
get_process_ppid
read_lock_pid
acquire_exclusive_lock
ensure_pve_mod_directory_exists
@ -102,6 +103,24 @@ sub is_process_alive {
return 1;
}
# Returns the parent PID of $pid, or undef if it can't be determined.
sub get_process_ppid {
my ($pid) = @_;
return undef unless open my $fh, '<', "/proc/$pid/stat";
my $line = <$fh>;
close $fh;
return undef unless defined $line;
# comm field (2nd, in parens) can itself contain ')', so split on the
# last one before parsing the remaining space-separated fields.
my $last_paren = rindex($line, ')');
return undef if $last_paren < 0;
my @fields = split ' ', substr($line, $last_paren + 1);
return undef unless defined $fields[1] && $fields[1] =~ /^(\d+)$/;
return $1;
}
sub read_lock_pid {
my ($lock_path) = @_;