public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [RFC PATCH common] RESTEnvironment: better SIGCHLD handling in AnyEvent event loop
Date: Mon, 20 Feb 2023 11:08:28 +0100	[thread overview]
Message-ID: <20230220100828.3416873-1-d.csapak@proxmox.com> (raw)

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@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@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





             reply	other threads:[~2023-02-20 10:09 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-20 10:08 Dominik Csapak [this message]
2023-02-20 10:33 ` Wolfgang Bumiller
2023-03-07 17:58 ` [pve-devel] applied: " Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230220100828.3416873-1-d.csapak@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal