public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Laimer <h.laimer@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH pve-manager] pvescheduler: hold off HUP and CHLD while a child is forked and recorded
Date: Tue, 15 Sep 2026 15:36:36 +0200	[thread overview]
Message-ID: <20260915133636.858359-1-h.laimer@proxmox.com> (raw)

A HUP that lands between the fork and recording neither stops nor hands
over that child, since the hand-over string is built in the HUP handler
from what is recorded. A CHLD from a child that exits right after the
fork, before it is recorded, is missed by the reap it triggers and
cleared only at the next CHLD from an unrelated child.

Hold both signals off until the child is recorded, and once a HUP came
in fork nothing more, since the hand-over is already put together.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
really really tight timeframe, but these workers can be really fast if
nothing has to be done, either way, we should address this, even if
unlikely

 PVE/Service/pvescheduler.pm | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/PVE/Service/pvescheduler.pm b/PVE/Service/pvescheduler.pm
index f7271fc0..be33db6e 100755
--- a/PVE/Service/pvescheduler.pm
+++ b/PVE/Service/pvescheduler.pm
@@ -3,7 +3,7 @@ package PVE::Service::pvescheduler;
 use strict;
 use warnings;
 
-use POSIX qw(WNOHANG);
+use POSIX qw(WNOHANG SIGCHLD SIGHUP SIG_BLOCK SIG_SETMASK);
 
 use PVE::Jobs;
 use PVE::SafeSyslog;
@@ -80,10 +80,20 @@ sub run {
         # FIXME: some job types may handle this better themself or just not care - make configurable
         return if scalar(keys $self->{jobs}->{$type}->%*);
 
+        # recorded before a HUP hands children over or a CHLD reaps them
+        my $prev_mask = POSIX::SigSet->new();
+        POSIX::sigprocmask(SIG_BLOCK, POSIX::SigSet->new(SIGHUP, SIGCHLD), $prev_mask);
+        # the hand-over is already built once a HUP came in
+        if ($self->{got_hup_signal}) {
+            POSIX::sigprocmask(SIG_SETMASK, $prev_mask);
+            return;
+        }
         my $child = fork();
         if (!defined($child)) {
+            POSIX::sigprocmask(SIG_SETMASK, $prev_mask);
             die "fork failed: $!\n";
         } elsif ($child == 0) {
+            POSIX::sigprocmask(SIG_SETMASK, $prev_mask);
             $self->after_fork_cleanup();
             eval { $sub->(); };
             if (my $err = $@) {
@@ -93,6 +103,7 @@ sub run {
         }
 
         $jobs->{$type}->{$child} = 1;
+        POSIX::sigprocmask(SIG_SETMASK, $prev_mask);
     };
 
     my $first_run = 1;
-- 
2.47.3





                 reply	other threads:[~2026-09-15 13:36 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260915133636.858359-1-h.laimer@proxmox.com \
    --to=h.laimer@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal