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,