mirror of
https://github.com/xcat2/xcat-dep.git
synced 2026-09-12 12:36:23 +00:00
fix(xcat-dep): block cancellation across worker registration
The forwarding handler was in place before the fork, but the parent recorded the worker pid after it. A signal in between reached a handler that did not know the worker, so the orchestrator died while the worker kept building for hours and held the per-architecture lock. The mask discipline run_bounded already used is now a pair of helpers, and the worker loop blocks the handled signals across both the fork and the registration. The worker resets the inherited handlers before restoring the mask.
This commit is contained in:
+24
-6
@@ -21,6 +21,8 @@ our @EXPORT_OK = qw(
|
||||
display_quote
|
||||
every_step_failed
|
||||
forward_signals_to_workers
|
||||
block_handled_signals
|
||||
restore_signal_mask
|
||||
hashes_equal
|
||||
print_step
|
||||
read_binary
|
||||
@@ -200,6 +202,24 @@ sub stall_report {
|
||||
return $total;
|
||||
}
|
||||
|
||||
# block_handled_signals(): block INT, TERM and HUP and return the previous mask, for the window
|
||||
# between forking a child and being able to signal it. A cancellation arriving in that window would
|
||||
# otherwise kill the parent under a handler that does not know the child yet, and the child would
|
||||
# keep running. A blocked signal stays pending and is delivered by restore_signal_mask().
|
||||
sub block_handled_signals {
|
||||
my $handled = POSIX::SigSet->new(POSIX::SIGINT(), POSIX::SIGTERM(), POSIX::SIGHUP());
|
||||
my $previous = POSIX::SigSet->new();
|
||||
POSIX::sigprocmask(POSIX::SIG_BLOCK(), $handled, $previous);
|
||||
return $previous;
|
||||
}
|
||||
|
||||
# restore_signal_mask($previous): put the mask back, delivering anything that arrived meanwhile.
|
||||
sub restore_signal_mask {
|
||||
my ($previous) = @_;
|
||||
POSIX::sigprocmask(POSIX::SIG_SETMASK(), $previous) if $previous;
|
||||
return;
|
||||
}
|
||||
|
||||
# forward_signals_to_workers(%a): return an INT/TERM/HUP handler that passes the signal on to the
|
||||
# forked workers, waits for them, then re-raises it. An orchestrator that dies without this releases
|
||||
# its locks while its workers keep building and writing into staging, and the next run races
|
||||
@@ -239,18 +259,16 @@ sub run_bounded {
|
||||
# handler below would kill this process under the inherited handler and leave the new process
|
||||
# group running. A blocked signal stays pending and is delivered once the handler is in place.
|
||||
# The child restores the mask before exec, or the build would inherit a blocked TERM.
|
||||
my $handled = POSIX::SigSet->new(POSIX::SIGINT(), POSIX::SIGTERM(), POSIX::SIGHUP());
|
||||
my $previous = POSIX::SigSet->new();
|
||||
POSIX::sigprocmask(POSIX::SIG_BLOCK(), $handled, $previous);
|
||||
my $previous = block_handled_signals();
|
||||
|
||||
my $pid = fork();
|
||||
unless (defined $pid) {
|
||||
POSIX::sigprocmask(POSIX::SIG_SETMASK(), $previous);
|
||||
restore_signal_mask($previous);
|
||||
die "run_bounded: fork failed: $!\n";
|
||||
}
|
||||
if ($pid == 0) {
|
||||
POSIX::setpgid(0, 0);
|
||||
POSIX::sigprocmask(POSIX::SIG_SETMASK(), $previous);
|
||||
restore_signal_mask($previous);
|
||||
exec('bash', '-c', $cmd) or POSIX::_exit(127);
|
||||
}
|
||||
# setpgid from BOTH sides: whichever runs first wins, so the group exists before the first signal
|
||||
@@ -279,7 +297,7 @@ sub run_bounded {
|
||||
local $SIG{INT} = $forward;
|
||||
local $SIG{TERM} = $forward;
|
||||
local $SIG{HUP} = $forward;
|
||||
POSIX::sigprocmask(POSIX::SIG_SETMASK(), $previous);
|
||||
restore_signal_mask($previous);
|
||||
|
||||
# An unbounded run still forks: it is the process group, not the deadline, that lets a signal
|
||||
# to the orchestrator reach the build.
|
||||
|
||||
+10
-1
@@ -641,15 +641,24 @@ sub build_deps {
|
||||
while (@queue || $running) {
|
||||
while (@queue && $running < $max) {
|
||||
my $cn = shift @queue;
|
||||
# Block the handled signals across the fork AND the registration below: a cancellation
|
||||
# in between would reach a handler that does not know this worker yet, and the worker
|
||||
# would keep building for hours while holding the per-arch lock.
|
||||
my $previous = XCAT::BuildUtils::block_handled_signals();
|
||||
my $pid = fork();
|
||||
die "FATAL: fork failed: $!\n" unless defined $pid;
|
||||
unless (defined $pid) {
|
||||
XCAT::BuildUtils::restore_signal_mask($previous);
|
||||
die "FATAL: fork failed: $!\n";
|
||||
}
|
||||
# The child must not inherit the parent's forwarder: its copy names sibling workers,
|
||||
# which the parent already signals.
|
||||
if ($pid == 0) {
|
||||
$SIG{$_} = 'DEFAULT' for qw(INT TERM HUP);
|
||||
XCAT::BuildUtils::restore_signal_mask($previous);
|
||||
exit(build_one_codename($cn));
|
||||
}
|
||||
$pid2cn{$pid} = $cn; $running++;
|
||||
XCAT::BuildUtils::restore_signal_mask($previous);
|
||||
}
|
||||
my $pid = wait();
|
||||
if ($pid > 0) {
|
||||
|
||||
Reference in New Issue
Block a user