mirror of
https://github.com/webmin/webmin.git
synced 2026-09-16 10:30:39 +01:00
Add WebminCron pause during module post-install
This PR stops Miniserv from starting scheduled jobs while run-postinstalls.pl is running, so jobs never execute against half-updated module code or config. Miniserv defers all jobs, including startup jobs, while any marker belongs to a live process, and discards markers left by runners that died or that are older than an hour. The pause is best-effort, as a failure to create the marker is reported but never blocks post-installs. By the way, this issue was not reported by anyone. I discovered it while running many other extensive tests.
This commit is contained in:
Ilia Ross
@@ -10,6 +10,10 @@ eval "use Time::HiRes;";
|
||||
@itoa64 = split(//, "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz");
|
||||
@miniserv_argv = @ARGV;
|
||||
|
||||
# Limit scheduler pauses so an interrupted update or reused PID cannot block
|
||||
# WebminCron jobs indefinitely.
|
||||
$webmincron_pause_max_age = 60*60;
|
||||
|
||||
# init days and months for http_date
|
||||
@weekday = ( "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat" );
|
||||
@month = ( "Jan", "Feb", "Mar", "Apr", "May", "Jun",
|
||||
@@ -3975,6 +3979,7 @@ if (!$config{'webmincron_wrapper'}) {
|
||||
$config{'webmincron_wrapper'} = $config{'root'}.
|
||||
"/webmincron/webmincron.pl";
|
||||
}
|
||||
$config{'webmincron_pause'} ||= $var_dir."/webmincron-pause";
|
||||
if (!$config{'twofactor_wrapper'}) {
|
||||
$config{'twofactor_wrapper'} = $config{'root'}."/acl/twofactor.pl";
|
||||
}
|
||||
@@ -5833,12 +5838,44 @@ foreach $k (keys %{$_[1]}) {
|
||||
close(ARFILE);
|
||||
}
|
||||
|
||||
# webmincrons_paused()
|
||||
# Returns true while an unexpired marker has a live PID.
|
||||
# Removes expired markers and markers for dead PIDs.
|
||||
sub webmincrons_paused
|
||||
{
|
||||
my $pause_dir = $config{'webmincron_pause'}.".d";
|
||||
opendir(my $pause, $pause_dir) || return 0;
|
||||
my $paused = 0;
|
||||
my $now = time();
|
||||
foreach my $marker (readdir($pause)) {
|
||||
next if ($marker !~ /^(\d+)$/);
|
||||
my $pid = $1;
|
||||
my $file = "$pause_dir/$marker";
|
||||
my @st = stat($file);
|
||||
if (@st && $now - $st[9] > $webmincron_pause_max_age) {
|
||||
# Enforce the maximum pause even if the PID remains live or has
|
||||
# been reused.
|
||||
unlink($file);
|
||||
}
|
||||
# EPERM means the PID exists but Miniserv cannot signal it.
|
||||
elsif (kill(0, $pid) || $!{'EPERM'}) {
|
||||
$paused = 1;
|
||||
}
|
||||
else {
|
||||
unlink($file);
|
||||
}
|
||||
}
|
||||
closedir($pause);
|
||||
return $paused;
|
||||
}
|
||||
|
||||
# execute_ready_webmin_crons(run-count)
|
||||
# Find and run any cron jobs that are due, based on their last run time and
|
||||
# execution interval
|
||||
# Runs due WebminCron jobs. Returns false while paused, preserving the startup
|
||||
# pass for later.
|
||||
sub execute_ready_webmin_crons
|
||||
{
|
||||
my ($runs) = @_;
|
||||
return 0 if (&webmincrons_paused());
|
||||
my $now = time();
|
||||
my $changed = 0;
|
||||
foreach my $cron (@webmincrons) {
|
||||
@@ -5890,6 +5927,7 @@ if ($changed) {
|
||||
# Write out file containing last run times
|
||||
&write_file($config{'webmincron_last'}, \%webmincron_last);
|
||||
}
|
||||
return 1;
|
||||
}
|
||||
|
||||
# matches_cron(cron-spec, time, first-value)
|
||||
|
||||
@@ -631,7 +631,7 @@ while(1) {
|
||||
}
|
||||
|
||||
# Check if any webmin cron jobs are ready to run
|
||||
&execute_ready_webmin_crons($cron_runs++);
|
||||
$cron_runs++ if (&execute_ready_webmin_crons($cron_runs));
|
||||
|
||||
# wait for a new connection, or a message from a child process
|
||||
local ($i, $rmask);
|
||||
|
||||
@@ -9,6 +9,43 @@ use WebminCore;
|
||||
&init_config();
|
||||
$current_theme = $WebminCore::current_theme = undef;
|
||||
|
||||
# Pause WebminCron jobs while module post-install scripts run.
|
||||
# Each runner uses its own marker.
|
||||
my %miniserv;
|
||||
&get_miniserv_config(\%miniserv);
|
||||
# Use the same default path as Miniserv.
|
||||
my $miniserv_var_dir = $var_directory;
|
||||
$miniserv_var_dir = $1
|
||||
if ($miniserv{'pidfile'} &&
|
||||
$miniserv{'pidfile'} =~ /^(.*)\/[^\/]+$/);
|
||||
my $webmincron_pause_file = $miniserv{'webmincron_pause'} ||
|
||||
$miniserv_var_dir."/webmincron-pause";
|
||||
my $webmincron_pause_dir = $webmincron_pause_file.".d";
|
||||
my $webmincron_pause_marker = $webmincron_pause_dir."/".$$;
|
||||
my $webmincron_pause_pid = $$;
|
||||
my $webmincron_paused;
|
||||
if (!-d $webmincron_pause_dir &&
|
||||
!mkdir($webmincron_pause_dir, 0700) &&
|
||||
!-d $webmincron_pause_dir) {
|
||||
print STDERR "Cannot pause scheduled jobs: failed to create " .
|
||||
"$webmincron_pause_dir: $!\n";
|
||||
}
|
||||
elsif (open(my $pause, ">", $webmincron_pause_marker)) {
|
||||
close($pause);
|
||||
$webmincron_paused = 1;
|
||||
}
|
||||
else {
|
||||
print STDERR "Cannot pause scheduled jobs: failed to create " .
|
||||
"$webmincron_pause_marker: $!\n";
|
||||
}
|
||||
# Forked children also run the END block, so only the owner removes its marker.
|
||||
# Keep the shared directory for concurrent runners.
|
||||
END {
|
||||
if ($webmincron_paused && $$ == $webmincron_pause_pid) {
|
||||
unlink($webmincron_pause_marker);
|
||||
}
|
||||
}
|
||||
|
||||
if (@ARGV > 0) {
|
||||
# Running for specified modules
|
||||
foreach my $a (@ARGV) {
|
||||
@@ -37,4 +74,3 @@ foreach my $m (@mods) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
119
t/miniserv.t
119
t/miniserv.t
@@ -251,6 +251,125 @@ subtest 'prefix_to_mask' => sub {
|
||||
is(miniserv::prefix_to_mask(32), '255.255.255.255', '/32 = all ones');
|
||||
};
|
||||
|
||||
# webmincrons_paused — post-install pause markers
|
||||
subtest 'webmincrons_paused' => sub {
|
||||
my $pause = File::Spec->catfile(File::Spec->tmpdir(),
|
||||
"webmincron-pause-$$");
|
||||
my $pause_dir = $pause.'.d';
|
||||
my $default_pause = File::Spec->catfile(File::Spec->tmpdir(),
|
||||
"webmincron-pause");
|
||||
{
|
||||
no warnings 'once';
|
||||
local %miniserv::config = (
|
||||
'pidfile' => File::Spec->catfile(File::Spec->tmpdir(),
|
||||
"miniserv-$$.pid"),
|
||||
'logfile' => File::Spec->catfile(File::Spec->tmpdir(),
|
||||
"miniserv-$$.log"),
|
||||
);
|
||||
local $miniserv::config_file = File::Spec->catfile(
|
||||
File::Spec->tmpdir(), "miniserv-$$.conf");
|
||||
miniserv::update_vital_config();
|
||||
is($miniserv::config{'webmincron_pause'}, $default_pause,
|
||||
'pause path defaults to the Miniserv variable directory');
|
||||
}
|
||||
local $miniserv::config{'webmincron_pause'} = $pause;
|
||||
|
||||
rmdir($pause_dir);
|
||||
ok(!miniserv::webmincrons_paused(),
|
||||
'scheduler is not paused without markers');
|
||||
|
||||
mkdir($pause_dir, 0700) or die "create pause directory: $!";
|
||||
my $live_marker = File::Spec->catfile($pause_dir, $$);
|
||||
open(my $live, '>', $live_marker) or die "open pause marker: $!";
|
||||
close($live) or die "close pause marker: $!";
|
||||
my $stale_marker = File::Spec->catfile($pause_dir, 2147483647);
|
||||
open(my $stale, '>', $stale_marker) or die "open pause marker: $!";
|
||||
close($stale) or die "close pause marker: $!";
|
||||
ok(miniserv::webmincrons_paused(),
|
||||
'live marker pauses scheduled jobs');
|
||||
ok(!-e $stale_marker, 'stale markers are removed while jobs are paused');
|
||||
|
||||
unlink($live_marker);
|
||||
open($stale, '>', $stale_marker) or die "open pause marker: $!";
|
||||
close($stale) or die "close pause marker: $!";
|
||||
ok(!miniserv::webmincrons_paused(),
|
||||
'stale pause marker is ignored');
|
||||
ok(!-e $stale_marker, 'stale pause marker is removed');
|
||||
|
||||
# A marker past the age limit is stale even though its PID is alive.
|
||||
{
|
||||
no warnings 'once';
|
||||
open($live, '>', $live_marker) or die "open pause marker: $!";
|
||||
close($live) or die "close pause marker: $!";
|
||||
my $old = time() - $miniserv::webmincron_pause_max_age - 1;
|
||||
utime($old, $old, $live_marker) or die "age pause marker: $!";
|
||||
ok(!miniserv::webmincrons_paused(),
|
||||
'expired pause marker is ignored');
|
||||
ok(!-e $live_marker, 'expired pause marker is removed');
|
||||
}
|
||||
|
||||
# Pausing delays startup jobs instead of skipping them.
|
||||
my $last = $pause.'-last';
|
||||
my $started = 0;
|
||||
{
|
||||
no warnings qw(once redefine);
|
||||
local @miniserv::webmincrons = ({
|
||||
'id' => 'test', 'boot' => 1, 'module' => 'test',
|
||||
'func' => 'test',
|
||||
});
|
||||
local %miniserv::webmincron_last = ();
|
||||
local @miniserv::childpids = ();
|
||||
local $miniserv::config{'webmincron_last'} = $last;
|
||||
local *miniserv::execute_webmin_command = sub {
|
||||
$started++;
|
||||
return 12345;
|
||||
};
|
||||
|
||||
open(my $active, '>', $live_marker)
|
||||
or die "open pause marker: $!";
|
||||
close($active) or die "close pause marker: $!";
|
||||
ok(!miniserv::execute_ready_webmin_crons(0),
|
||||
'scheduler defers jobs while paused');
|
||||
is($started, 0, 'paused scheduler starts no jobs');
|
||||
is_deeply(\%miniserv::webmincron_last, {},
|
||||
'paused scheduler preserves job history');
|
||||
ok(!-e $last, 'paused scheduler does not write job history');
|
||||
|
||||
unlink($live_marker);
|
||||
ok(miniserv::execute_ready_webmin_crons(0),
|
||||
'scheduler resumes after marker removal');
|
||||
is($started, 1, 'deferred startup job runs after marker removal');
|
||||
ok(miniserv::execute_ready_webmin_crons(1),
|
||||
'subsequent scheduler passes complete');
|
||||
is($started, 1, 'startup job does not run again');
|
||||
|
||||
# Overdue interval jobs retain their due time until the pause ends.
|
||||
local @miniserv::webmincrons = ({
|
||||
'id' => 'interval', 'interval' => 60, 'module' => 'test',
|
||||
'func' => 'test',
|
||||
});
|
||||
my $previous = time() - 120;
|
||||
local %miniserv::webmincron_last = ('interval' => $previous);
|
||||
open($active, '>', $live_marker)
|
||||
or die "open pause marker: $!";
|
||||
close($active) or die "close pause marker: $!";
|
||||
ok(!miniserv::execute_ready_webmin_crons(1),
|
||||
'scheduler defers overdue interval jobs while paused');
|
||||
is($started, 1, 'paused interval job does not start');
|
||||
is($miniserv::webmincron_last{'interval'}, $previous,
|
||||
'paused interval job retains its last run time');
|
||||
unlink($live_marker);
|
||||
ok(miniserv::execute_ready_webmin_crons(1),
|
||||
'scheduler resumes overdue interval jobs');
|
||||
is($started, 2, 'overdue interval job runs after marker removal');
|
||||
}
|
||||
|
||||
unlink($live_marker);
|
||||
unlink($stale_marker);
|
||||
rmdir($pause_dir);
|
||||
unlink($last);
|
||||
};
|
||||
|
||||
# check_ipaddress / check_ip6address — input validators
|
||||
subtest 'check_ipaddress' => sub {
|
||||
ok( miniserv::check_ipaddress('1.2.3.4'), 'valid IPv4 accepted');
|
||||
|
||||
134
t/run-postinstalls.t
Normal file
134
t/run-postinstalls.t
Normal file
@@ -0,0 +1,134 @@
|
||||
#!/usr/bin/perl
|
||||
# Tests scheduler pause markers around module post-install scripts.
|
||||
|
||||
use strict;
|
||||
use warnings;
|
||||
use Test::More;
|
||||
use Cwd qw(abs_path);
|
||||
use File::Basename qw(dirname);
|
||||
use File::Path qw(make_path);
|
||||
use File::Spec;
|
||||
use File::Temp qw(tempdir);
|
||||
use IPC::Open3;
|
||||
use Symbol qw(gensym);
|
||||
|
||||
my $root = abs_path(File::Spec->catdir(dirname(__FILE__), '..'));
|
||||
my $runner = File::Spec->catfile($root, 'run-postinstalls.pl');
|
||||
my $tmp = tempdir(CLEANUP => 1);
|
||||
my $config_dir = File::Spec->catdir($tmp, 'config');
|
||||
my $var_dir = File::Spec->catdir($tmp, 'var');
|
||||
my $extra_root = File::Spec->catdir($tmp, 'root');
|
||||
my $module_dir = File::Spec->catdir($extra_root, 'pause-test');
|
||||
my $sentinel = File::Spec->catfile($tmp, 'postinstall-ran');
|
||||
make_path($config_dir, $var_dir, $module_dir);
|
||||
|
||||
write_text(File::Spec->catfile($config_dir, 'config'), "");
|
||||
write_text(File::Spec->catfile($module_dir, 'module.info'),
|
||||
"name=Pause test\ndesc=Pause test\n");
|
||||
write_text(File::Spec->catfile($module_dir, 'postinstall.pl'), <<'EOF');
|
||||
sub module_install
|
||||
{
|
||||
# A child that exits without exec also runs the runner's END block. It must not
|
||||
# remove the parent's marker.
|
||||
my $child = fork();
|
||||
die "fork: $!" if (!defined($child));
|
||||
exit(0) if (!$child);
|
||||
waitpid($child, 0);
|
||||
open(my $sentinel, ">", $ENV{'POSTINSTALL_SENTINEL'}) ||
|
||||
die "create post-install sentinel: $!";
|
||||
my $marker = $ENV{'POSTINSTALL_PAUSE'}.".d/".$$;
|
||||
my $state = -e $marker ? "paused\n" : "unpaused\n";
|
||||
print $sentinel $state;
|
||||
close($sentinel) || die "close post-install sentinel: $!";
|
||||
}
|
||||
1;
|
||||
EOF
|
||||
|
||||
sub write_text
|
||||
{
|
||||
my ($file, $text) = @_;
|
||||
open(my $fh, '>', $file) || die "create $file: $!";
|
||||
print $fh $text;
|
||||
close($fh) || die "close $file: $!";
|
||||
}
|
||||
|
||||
sub run_postinstalls
|
||||
{
|
||||
my ($pause, $set_pause) = @_;
|
||||
my $pause_config = $set_pause ? "webmincron_pause=$pause\n" : "";
|
||||
write_text(File::Spec->catfile($config_dir, 'miniserv.conf'),
|
||||
"root=$root\n".
|
||||
"extraroot_0=$extra_root\n".
|
||||
"pidfile=$var_dir/miniserv.pid\n".
|
||||
"logfile=$var_dir/miniserv.log\n".
|
||||
$pause_config);
|
||||
unlink($sentinel);
|
||||
|
||||
local $ENV{'WEBMIN_CONFIG'} = $config_dir;
|
||||
local $ENV{'WEBMIN_VAR'} = $var_dir;
|
||||
local $ENV{'POSTINSTALL_SENTINEL'} = $sentinel;
|
||||
local $ENV{'POSTINSTALL_PAUSE'} = $pause;
|
||||
my $stderr = gensym();
|
||||
my $pid = open3(undef, my $stdout, $stderr, $^X, $runner, 'pause-test');
|
||||
local $/;
|
||||
my $out = <$stdout> // '';
|
||||
my $err = <$stderr> // '';
|
||||
waitpid($pid, 0);
|
||||
return ($? >> 8, $out, $err);
|
||||
}
|
||||
|
||||
my $pause = File::Spec->catfile($var_dir, 'webmincron-pause');
|
||||
my ($status, $output, $error) = run_postinstalls($pause, 0);
|
||||
is($status, 0, 'post-install runner exits successfully');
|
||||
is(read_text($sentinel), "paused\n",
|
||||
'forked child does not remove the runner\'s marker');
|
||||
ok(-d $pause.'.d', 'pause directory remains after the runner exits');
|
||||
is_deeply([ numeric_markers($pause.'.d') ], [],
|
||||
'post-install marker is removed on exit');
|
||||
is($error, '', 'successful pause setup reports no error');
|
||||
|
||||
# A runner using a custom path must leave another runner's marker intact.
|
||||
my $custom_pause = File::Spec->catfile($var_dir, 'custom-pause');
|
||||
my $custom_dir = $custom_pause.'.d';
|
||||
make_path($custom_dir);
|
||||
my $other_marker = File::Spec->catfile($custom_dir, $$);
|
||||
write_text($other_marker, "");
|
||||
($status, $output, $error) = run_postinstalls($custom_pause, 1);
|
||||
is($status, 0, 'runner succeeds with a configured pause path');
|
||||
is(read_text($sentinel), "paused\n", 'runner uses the configured pause path');
|
||||
is_deeply([ numeric_markers($custom_dir) ], [ "$$" ],
|
||||
'runner removes only its own marker');
|
||||
is($error, '', 'overlapping pause markers report no error');
|
||||
unlink($other_marker);
|
||||
|
||||
# Failure to create the shared directory must not skip post-install scripts.
|
||||
my $blocker = File::Spec->catfile($tmp, 'not-a-directory');
|
||||
write_text($blocker, "");
|
||||
my $invalid_pause = File::Spec->catfile($blocker, 'webmincron-pause');
|
||||
($status, $output, $error) = run_postinstalls($invalid_pause, 1);
|
||||
is($status, 0, 'pause setup failure does not stop the runner');
|
||||
is(read_text($sentinel), "unpaused\n",
|
||||
'module post-install still runs when pause setup fails');
|
||||
like($error, qr/^Cannot pause scheduled jobs:/m,
|
||||
'pause setup failure is reported');
|
||||
|
||||
sub read_text
|
||||
{
|
||||
my ($file) = @_;
|
||||
open(my $fh, '<', $file) || die "open $file: $!";
|
||||
local $/;
|
||||
my $text = <$fh>;
|
||||
close($fh);
|
||||
return $text;
|
||||
}
|
||||
|
||||
sub numeric_markers
|
||||
{
|
||||
my ($dir) = @_;
|
||||
opendir(my $dh, $dir) || die "open $dir: $!";
|
||||
my @markers = grep { /^\d+$/ } readdir($dh);
|
||||
closedir($dh);
|
||||
return @markers;
|
||||
}
|
||||
|
||||
done_testing();
|
||||
Reference in New Issue
Block a user