[pve-devel] [RFC PATCH common] RESTEnvironment: better SIGCHLD handling in AnyEvent event loop

[Dominik Csapak]

when we're in an API server that uses AnyEvent, we must postpone
the worker_reaper, since it calls 'active_workers' which might already
be called and then we're inside the lock twice (flocks are per process
for us, see PVE::Tools::lock_file)

This resulted in an error like this:
close (rename) atomic file '/var/log/pve/tasks/active' failed: No such file or directory

We use the fact that only 'pub' and 'priv' RESTEnvironment types are an
api server with anyevent. For other types we call it like before.

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
Not super happy about the coupling between the RESTEnvironment and AnyEvent.
We could try to just save the worker_reaper in 'self' and let the users
of the env decide when to call it, but that would be more involved.

OTOH, we already do some anyevent specific things in PVE::Daemon
(without depending on the AnyEvent package though)...

Also i did not find a way to dynamically find out if we're in an
AnyEvent loop...

 debian/control             |  1 +
 src/PVE/RESTEnvironment.pm | 13 ++++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/debian/control b/debian/control
index 232a0e4..1c75985 100644
--- a/debian/control
+++ b/debian/control
@@ -3,6 +3,7 @@ Section: perl
 Priority: optional
 Maintainer: Proxmox Support Team <support at proxmox.com>
 Build-Depends: debhelper (>= 12~),
+               libanyevent-perl,
                libclone-perl,
                libdevel-cycle-perl,
                libfilesys-df-perl,
diff --git a/src/PVE/RESTEnvironment.pm b/src/PVE/RESTEnvironment.pm
index bf89c12..c258b1e 100644
--- a/src/PVE/RESTEnvironment.pm
+++ b/src/PVE/RESTEnvironment.pm
@@ -14,6 +14,7 @@ use IO::File;
 use IO::Handle;
 use IO::Select;
 use POSIX qw(:sys_wait_h EINTR);
+use AnyEvent;
 
 use PVE::Exception qw(raise raise_perm_exc);
 use PVE::INotify;
@@ -111,7 +112,17 @@ sub init {
     die "unknown environment type"
 	if !$type || $type !~ m/^(cli|pub|priv|ha)$/;
 
-    $SIG{CHLD} = $worker_reaper;
+    my $has_anyevent = $type eq 'pub' || $type eq 'priv';
+
+    $SIG{CHLD} = sub {
+	# when we're in an api server, we have to postpone the call to worker_reaper, otherwise it
+	# might interfere with running api calls
+	if ($has_anyevent) {
+	    AnyEvent::postpone { $worker_reaper->() };
+	} else {
+	    $worker_reaper->();
+	}
+    };
 
     # environment types
     # cli  ... command started fron command line
-- 
2.30.2