From 5ac62ebb5100dd0c02980311f2f5c792a868b2e1 Mon Sep 17 00:00:00 2001
From: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Date: Thu, 24 Sep 2026 00:45:03 -0300
Subject: [PATCH] fix(xcat-core): a queued install monitor request waits for
 the next unrelated node

The install monitor holds the later connections for a node whose handler
is still running, and forks the next one when it reaps that handler.
SIGCHLD is what brings the parent back to look: it interrupts the accept.

A handler can exit after the parent checks its queue and before the
accept begins. The signal is then handled where there is no accept to
interrupt, and the parent blocks in accept with a connection already
queued and ready to run. That connection waits until some other node
calls in. A single node retrying on its own waits until it times out.

do_installm_service now waits through wait_for_installm_connection, which
selects on the listening socket. The wait is bounded by
$installm_wakeup_seconds while connections are queued, so the parent
looks at its queue again instead of waiting for another client. An idle
monitor with an empty queue still waits without a bound, because a
handler that exits then leaves nothing to do.

The new case asserts the wait ends on its own bound with nothing to
accept, and ends at once when a connection is already there. It fails
when the bound is ignored.

Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
---
 xCAT-server/sbin/xcatd                        | 38 +++++++++++++++
 .../unit/xcatd_install_monitor_concurrency.t  | 46 +++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/xCAT-server/sbin/xcatd b/xCAT-server/sbin/xcatd
index 53b247313..3b7d09658 100755
--- a/xCAT-server/sbin/xcatd
+++ b/xCAT-server/sbin/xcatd
@@ -353,6 +353,11 @@ my $installm_maxkids = 64;
 # Under the systemd TimeoutStopSec of 30 seconds, so the drain finishes before the kill.
 my $installm_drain_seconds = 20;
 
+# How long the parent waits for a connection while it still holds queued ones. SIGCHLD only
+# wakes a wait that is already running, so a handler that exits just before the wait begins
+# interrupts nothing. The bound is the parent's own second look.
+my $installm_wakeup_seconds = 1;
+
 # One path, so a test can point the lifted routine somewhere else.
 my $installm_pidfile = "/var/run/xcat/installservice.pid";
 
@@ -396,6 +401,34 @@ sub dequeue_installm_request {
     return ();
 }
 
+# --------------------------------------------------------------------------------
+
+=head3 wait_for_installm_connection
+
+    Descriptions:
+        Wait until the install monitor has a connection to accept.
+    Arguments:
+        socket:  the listening socket
+        seconds: how long to wait, or undef to wait until a connection arrives
+    Returns:
+        1 - the socket has a connection to accept
+        0 - the wait ended without one, so the caller looks at its queue again
+=cut
+
+# --------------------------------------------------------------------------------
+sub wait_for_installm_connection {
+    my ($socket, $seconds) = @_;
+
+    # USR2 closes the socket under the monitor. There is nothing to wait for then.
+    my $fd = fileno($socket);
+    return 0 unless defined $fd;
+
+    my $waiting = '';
+    vec($waiting, $fd, 1) = 1;
+    my $ready = select($waiting, undef, undef, $seconds);
+    return (defined $ready and $ready > 0) ? 1 : 0;
+}
+
 sub do_installm_service {
     unless ($sport) { return; }
 
@@ -476,6 +509,11 @@ sub do_installm_service {
 
         # Nothing was queued, so take a new connection and name its peer.
         unless (defined $conn) {
+            # A handler can exit between the check above and the accept below, where the
+            # SIGCHLD has no accept to interrupt. Bound the wait while connections are queued,
+            # so the parent looks again instead of waiting for the next unrelated client.
+            next unless wait_for_installm_connection($socket,
+                %installm_queue ? $installm_wakeup_seconds : undef);
             next unless $conn = $socket->accept;
             eval {
                 # check if a rescanplugins request has come in
diff --git a/xCAT-test/unit/xcatd_install_monitor_concurrency.t b/xCAT-test/unit/xcatd_install_monitor_concurrency.t
index f15ba809f..f0494fdc7 100644
--- a/xCAT-test/unit/xcatd_install_monitor_concurrency.t
+++ b/xCAT-test/unit/xcatd_install_monitor_concurrency.t
@@ -57,6 +57,8 @@ my $reaper = lift_sub('reap_installm_kids')
   or die "xcatd no longer defines reap_installm_kids";
 my $dequeue = lift_sub('dequeue_installm_request')
   or die "xcatd no longer defines dequeue_installm_request";
+my $waiter = lift_sub('wait_for_installm_connection')
+  or die "xcatd no longer defines wait_for_installm_connection";
 
 # Settings the lifted routine reads from file-scope variables xcatd declares but this file does
 # not lift. Read the defaults out of the source, so a rename fails here instead of silently
@@ -65,6 +67,8 @@ my ($MAXKIDS) = $src =~ /^my \s+ \$installm_maxkids \s* = \s* (\d+) ;/mx;
 $MAXKIDS or die "xcatd no longer declares \$installm_maxkids";
 my ($DRAIN) = $src =~ /^my \s+ \$installm_drain_seconds \s* = \s* (\d+) ;/mx;
 $DRAIN or die "xcatd no longer declares \$installm_drain_seconds";
+my ($WAKEUP) = $src =~ /^my \s+ \$installm_wakeup_seconds \s* = \s* ([\d.]+) ;/mx;
+$WAKEUP or die "xcatd no longer declares \$installm_wakeup_seconds";
 my ($PIDFILE) = $src =~ /^my \s+ \$installm_pidfile \s* = \s* "([^"]+)" ;/mx;
 $PIDFILE or die "xcatd no longer declares \$installm_pidfile";
 
@@ -176,6 +180,7 @@ sub wait_for_event {
       '}',
       $reaper,
       $dequeue,
+      $waiter,
       $service,
       '1;';
     eval $scratch or die "cannot compile the lifted install monitor: $@";
@@ -224,6 +229,7 @@ sub start_monitor {
     no warnings 'once';
     $t::installm::installm_maxkids       = $maxkids;
     $t::installm::installm_drain_seconds = $DRAIN;
+    $t::installm::installm_wakeup_seconds = $WAKEUP;
     $t::installm::installm_pidfile       = $SCRATCH_PIDFILE;
     $t::installm::sport                  = $port;
     $t::installm::quit                   = 0;
@@ -496,6 +502,46 @@ sub stop_monitor {
     stop_monitor($mon);
 }
 
+# --- the wait before the accept must end on its own ---------------------------
+
+# SIGCHLD only interrupts a wait that has already begun. A handler that exits between the queue
+# check and the wait leaves nothing to interrupt, so a wait that ends only when a connection
+# arrives strands the connection queued for that node until some other node calls in.
+{
+    my $listen = IO::Socket::INET->new(LocalAddr => '127.0.0.1', LocalPort => 0,
+        Listen => 1, Proto => 'tcp', ReuseAddr => 1)
+      or die "cannot open a listening socket: $!";
+
+    my $bounded;
+    eval {
+        local $SIG{ALRM} = sub { die "BLOCKED\n" };
+        alarm 10;
+        $bounded = t::installm::wait_for_installm_connection($listen, 0.2);
+        alarm 0;
+        1;
+    } or alarm 0;
+    is($@, '', 'the wait before the accept ends on its own bound with nothing to accept')
+      or diag('the wait ends only when a connection arrives, so a SIGCHLD handled before it is lost');
+    is($bounded, 0, 'and it reports that there is nothing to accept');
+
+    my $client = IO::Socket::INET->new(PeerAddr => '127.0.0.1',
+        PeerPort => $listen->sockport(), Proto => 'tcp', Timeout => 10);
+    ok($client, 'a client reached the listening socket');
+    my $ready;
+    eval {
+        local $SIG{ALRM} = sub { die "BLOCKED\n" };
+        alarm 10;
+        $ready = t::installm::wait_for_installm_connection($listen, $WAKEUP);
+        alarm 0;
+        1;
+    } or alarm 0;
+    is($ready, 1, 'a waiting connection ends the wait at once')
+      or diag('the monitor waits out its bound before accepting a connection that is already there');
+
+    close $client if $client;
+    close $listen;
+}
+
 # --- the per-node queue does not outlive the requests in it -------------------
 
 # An empty queue entry for every node ever served is a leak no behavioural assertion catches,