public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH librados2-perl 3/3] use POSIX::_exit instead of exit in forked child
Date: Tue,  8 Aug 2023 13:23:12 +0200	[thread overview]
Message-ID: <20230808112312.64667-4-f.ebner@proxmox.com> (raw)
In-Reply-To: <20230808112312.64667-1-f.ebner@proxmox.com>

This avoids calling the END blocks and destructor routines most of
which have nothing to do with the fork, so it cannot be assumed to be
safe to do from the fork.

Move the call to pve_rados_shutdown from the destructor routine,
which will not be called by the child anymore. Make sure the cleanup
is also done when the worker dies.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Better viewed with -w

 PVE/RADOS.pm | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/PVE/RADOS.pm b/PVE/RADOS.pm
index 41b9c39..f09d890 100644
--- a/PVE/RADOS.pm
+++ b/PVE/RADOS.pm
@@ -7,6 +7,7 @@ use warnings;
 
 use Carp;
 use JSON;
+use POSIX;
 use Socket;
 
 use PVE::Tools;
@@ -195,24 +196,30 @@ sub new {
     } else { # child
 	$0 = 'pverados';
 
-	PVE::INotify::inotify_close();
+	eval {
+	    PVE::INotify::inotify_close();
 
-	if (my $atfork = $rpcenv->{atfork}) {
-	    &$atfork();
-	}
+	    if (my $atfork = $rpcenv->{atfork}) {
+		&$atfork();
+	    }
 
-	# override signal handlers inherited from the parent
-	local $SIG{HUP} = $SIG{INT} = $SIG{QUIT} = $SIG{TERM} = sub {
-	    exit(1);
+	    # override signal handlers inherited from the parent
+	    local $SIG{HUP} = $SIG{INT} = $SIG{QUIT} = $SIG{TERM} = sub {
+		pve_rados_shutdown($self->{conn}) if $self->{conn};
+		POSIX::_exit(1);
+	    };
+
+	    # fixme: timeout?
+
+	    close $child;
+
+	    $self->pve_rados_work($parent, $timeout, %params);
 	};
+	my $err = $@;
+	warn $err if $err;
 
-	# fixme: timeout?
-
-	close $child;
-
-	$self->pve_rados_work($parent, $timeout, %params);
-
-	exit(0);
+	pve_rados_shutdown($self->{conn}) if $self->{conn};
+	POSIX::_exit($err ? 1 : 0);
     }
 
     return $self;
@@ -233,9 +240,6 @@ sub DESTROY {
 	#print "$$: DESTROY WAIT0\n";
 	&$kill_worker($self);
 	#print "$$: DESTROY WAIT\n";
-    } else {
-	#print "$$: DESTROY SHUTDOWN\n";
-	pve_rados_shutdown($self->{conn}) if $self->{conn};
     }
 }
 
-- 
2.39.2





  parent reply	other threads:[~2023-08-08 11:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-08 11:23 [pve-devel] [PATCH-SERIES librados2-perl] improve fork behavior Fiona Ebner
2023-08-08 11:23 ` [pve-devel] [PATCH librados2-perl 1/3] refactor pverados worker into dedicated function Fiona Ebner
2023-08-08 11:23 ` [pve-devel] [PATCH librados2-perl 2/3] reset inherited signal handlers in child worker Fiona Ebner
2023-08-08 11:23 ` Fiona Ebner [this message]
2023-08-11  7:13 ` [pve-devel] applied-series: [PATCH-SERIES librados2-perl] improve fork behavior Wolfgang Bumiller

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=20230808112312.64667-4-f.ebner@proxmox.com \
    --to=f.ebner@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