[pve-devel] [PATCH v2 pve-common 1/2] Tools::run_with_timeout improvement + hires alarm

[Wolfgang Bumiller]

The following situations could lead to the 'unknown error':
1) As commented, when the alarm triggered after the first
signal handler was installed and before the new alarm was
installed. In this case the $signalcount was increased,
and worse: the original signal handler was never called.

2) When $code died, since the call itself wasn't in an eval
block, we'd leave the eval block containing the inner alarm
signal handler. Then there's a time window from leaving the
signal block (and with that restoring the first installed
only-counting signal-handler) and reaching the code to
restore the previous alarm where the counting alarm handler
could get triggered by our own alarm set before running
$code. In this case at least the the old alarm would be
restored, but we'd still trigger the 'unknown error'.

The new code starts off by suspending the original alarm
before installing any signal handler, then installing the
timeout handler inside the first eval block. The $code is
then run inside another eval block to make sure we reach the
alarm(0) statement before restoring the old signal handler
and alarm timeout.
---
 src/PVE/Tools.pm | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 0c6dde6..577a8bc 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -18,7 +18,7 @@ use Encode;
 use Digest::SHA;
 use Text::ParseWords;
 use String::ShellQuote;
-use Time::HiRes qw(usleep gettimeofday tv_interval);
+use Time::HiRes qw(usleep gettimeofday tv_interval alarm);
 
 # avoid warning when parsing long hex values with hex()
 no warnings 'portable'; # Support for 64-bit ints required
@@ -68,30 +68,31 @@ sub run_with_timeout {
 
     die "got timeout\n" if $timeout <= 0;
 
-    my $prev_alarm;
+    my $prev_alarm = alarm 0; # suspend outer alarm early
 
     my $sigcount = 0;
 
     my $res;
 
-    local $SIG{ALRM} = sub { $sigcount++; }; # catch alarm outside eval
-
     eval {
 	local $SIG{ALRM} = sub { $sigcount++; die "got timeout\n"; };
 	local $SIG{PIPE} = sub { $sigcount++; die "broken pipe\n" };
 	local $SIG{__DIE__};   # see SA bug 4631
 
-	$prev_alarm = alarm($timeout);
+	alarm($timeout);
 
-	$res = &$code(@param);
+	eval { $res = &$code(@param); };
 
 	alarm(0); # avoid race conditions
+
+	die $@ if $@;
     };
 
     my $err = $@;
 
-    alarm($prev_alarm) if defined($prev_alarm);
+    alarm $prev_alarm;
 
+    # this shouldn't happen anymore?
     die "unknown error" if $sigcount && !$err; # seems to happen sometimes
 
     die $err if $err;
-- 
2.1.4