Adam Williamson
4 years ago
parent
6328fae92a
commit
f8b2c2bc52
@ -0,0 +1,46 @@
|
|||||||
|
From 3d16b5fb23c4a41ba15a1e8dfdcd2b302557c377 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Adam Williamson <awilliam@redhat.com>
|
||||||
|
Date: Tue, 13 Apr 2021 09:02:55 -0700
|
||||||
|
Subject: [PATCH] Re-connect to dbus for each call (POO #90872)
|
||||||
|
|
||||||
|
By using the system bus and leaving our connection to it open
|
||||||
|
for the entire lifetime of the isotovideo process, we're setting
|
||||||
|
ourselves up to receive a lot of signals we don't deal with, and
|
||||||
|
just letting them accumulate. Since all we want to do is send a
|
||||||
|
signal and then go away, let's use a private socket, and
|
||||||
|
re-initialize the connection for each signal. The overhead of
|
||||||
|
doing this is not large, and we're sending small numbers of
|
||||||
|
signals per isotovideo process anyway (one signal per network on
|
||||||
|
startup, one signal at startup if a debug variable is set, and
|
||||||
|
one signal per network at end).
|
||||||
|
|
||||||
|
Thanks to @dvdhrm and @berrange for their help with this.
|
||||||
|
|
||||||
|
Signed-off-by: Adam Williamson <awilliam@redhat.com>
|
||||||
|
---
|
||||||
|
backend/qemu.pm | 9 +++++----
|
||||||
|
1 file changed, 5 insertions(+), 4 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/backend/qemu.pm b/backend/qemu.pm
|
||||||
|
index 6c64f497..b847ddd1 100644
|
||||||
|
--- a/backend/qemu.pm
|
||||||
|
+++ b/backend/qemu.pm
|
||||||
|
@@ -135,10 +135,11 @@ sub stop_qemu {
|
||||||
|
|
||||||
|
sub _dbus_do_call {
|
||||||
|
my ($self, $fn, @args) = @_;
|
||||||
|
- $self->{dbus} ||= Net::DBus->system;
|
||||||
|
- $self->{dbus_service} ||= $self->{dbus}->get_service("org.opensuse.os_autoinst.switch");
|
||||||
|
- $self->{dbus_object} ||= $self->{dbus_service}->get_object("/switch", "org.opensuse.os_autoinst.switch");
|
||||||
|
- $self->{dbus_object}->$fn(@args);
|
||||||
|
+ my $bus = Net::DBus->system(private => "true");
|
||||||
|
+ my $bus_service = $bus->get_service("org.opensuse.os_autoinst.switch");
|
||||||
|
+ my $bus_object = $bus_service->get_object("/switch", "org.opensuse.os_autoinst.switch");
|
||||||
|
+ $bus_object->$fn(@args);
|
||||||
|
+ $bus->get_connection->disconnect;
|
||||||
|
}
|
||||||
|
|
||||||
|
sub _dbus_call {
|
||||||
|
--
|
||||||
|
2.31.1
|
||||||
|
|
||||||
@ -0,0 +1,108 @@
|
|||||||
|
From 61b1834c5366c78f0e4d3b21a142d7dd6d6322c1 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Fabian Vogt <fvogt@suse.de>
|
||||||
|
Date: Tue, 13 Apr 2021 15:41:00 +0200
|
||||||
|
Subject: [PATCH] signalblocker: Also block SIGCHLD
|
||||||
|
|
||||||
|
The perl-unaware threads created by OpenCV also crash when they encounter a
|
||||||
|
SIGCHLD signal, which happens by using perl's "system" function for instance.
|
||||||
|
---
|
||||||
|
signalblocker.pm | 22 +++++++++++-----------
|
||||||
|
t/28-signalblocker.t | 14 ++++++++++++--
|
||||||
|
2 files changed, 23 insertions(+), 13 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/signalblocker.pm b/signalblocker.pm
|
||||||
|
index 1ff3652c..dea56e12 100644
|
||||||
|
--- a/signalblocker.pm
|
||||||
|
+++ b/signalblocker.pm
|
||||||
|
@@ -1,4 +1,4 @@
|
||||||
|
-# Copyright © 2020 SUSE LLC
|
||||||
|
+# Copyright © 2021 SUSE LLC
|
||||||
|
#
|
||||||
|
# This program is free software; you can redistribute it and/or modify
|
||||||
|
# it under the terms of the GNU General Public License as published by
|
||||||
|
@@ -19,23 +19,23 @@ use Mojo::Base -base;
|
||||||
|
use bmwqemu;
|
||||||
|
use POSIX ':signal_h';
|
||||||
|
|
||||||
|
-# OpenCV forks a lot of threads and the TERM signal we may get from the
|
||||||
|
-# parent process would be delivered to an undefined thread. But as those
|
||||||
|
-# threads do not have a perl interpreter, the perl signal handler (we set
|
||||||
|
-# later) would crash. So we need to block the TERM signal in the forked
|
||||||
|
-# processes before we set the signal handler of our choice.
|
||||||
|
+# OpenCV forks a lot of threads and the signals we may get (TERM from the
|
||||||
|
+# parent, CHLD from children) would be delivered to an undefined thread.
|
||||||
|
+# But as those threads do not have a perl interpreter, the perl signal
|
||||||
|
+# handler would crash. We need to block those signals in those threads, so
|
||||||
|
+# that they get delivered only to those threads which can handle it.
|
||||||
|
|
||||||
|
sub new {
|
||||||
|
my ($class, @args) = @_;
|
||||||
|
|
||||||
|
# block signals
|
||||||
|
- bmwqemu::diag('Blocking SIGTERM');
|
||||||
|
+ bmwqemu::diag('Blocking SIGCHLD and SIGCHLD');
|
||||||
|
my %old_sig = %SIG;
|
||||||
|
$SIG{TERM} = 'IGNORE';
|
||||||
|
$SIG{INT} = 'IGNORE';
|
||||||
|
$SIG{HUP} = 'IGNORE';
|
||||||
|
- my $sigset = POSIX::SigSet->new(SIGTERM);
|
||||||
|
- die "Could not block SIGTERM\n" unless defined sigprocmask(SIG_BLOCK, $sigset, undef);
|
||||||
|
+ my $sigset = POSIX::SigSet->new(SIGCHLD, SIGTERM);
|
||||||
|
+ die "Could not block SIGCHLD and SIGTERM\n" unless defined sigprocmask(SIG_BLOCK, $sigset, undef);
|
||||||
|
|
||||||
|
# create the actual object holding the information to restore the previous state
|
||||||
|
my $self = $class->SUPER::new(@args);
|
||||||
|
@@ -48,8 +48,8 @@ sub DESTROY {
|
||||||
|
my ($self) = @_;
|
||||||
|
|
||||||
|
# set back signal handling to default to be able to terminate properly
|
||||||
|
- bmwqemu::diag('Unblocking SIGTERM');
|
||||||
|
- die "Could not unblock SIGTERM\n" unless defined sigprocmask(SIG_UNBLOCK, $self->{_sigset}, undef);
|
||||||
|
+ bmwqemu::diag('Unblocking SIGCHLD and SIGTERM');
|
||||||
|
+ die "Could not unblock SIGCHLD and SIGTERM\n" unless defined sigprocmask(SIG_UNBLOCK, $self->{_sigset}, undef);
|
||||||
|
%SIG = %{$self->{_old_sig}};
|
||||||
|
}
|
||||||
|
|
||||||
|
diff --git a/t/28-signalblocker.t b/t/28-signalblocker.t
|
||||||
|
index 31ddb6f6..9e1a99a5 100755
|
||||||
|
--- a/t/28-signalblocker.t
|
||||||
|
+++ b/t/28-signalblocker.t
|
||||||
|
@@ -1,6 +1,6 @@
|
||||||
|
#!/usr/bin/perl
|
||||||
|
#
|
||||||
|
-# Copyright (c) 2020 SUSE LLC
|
||||||
|
+# Copyright (c) 2021 SUSE LLC
|
||||||
|
#
|
||||||
|
# This program is free software; you can redistribute it and/or modify
|
||||||
|
# it under the terms of the GNU General Public License as published by
|
||||||
|
@@ -44,9 +44,13 @@ my $no_signal_blocker = $ENV{OS_AUTOINST_TEST_NO_SIGNAL_BLOCKER};
|
||||||
|
sub thread_count { scalar split("\n", `ps huH p $$`) }
|
||||||
|
is(my $last_thread_count = thread_count, 1, 'initially one thread');
|
||||||
|
|
||||||
|
-# count SIGTERMs we receive; this handler should work after creating/destroying the signal blocker
|
||||||
|
+# count SIGTERMs we receive; those handlers should work after creating/destroying the signal blocker
|
||||||
|
+# Note that without these handlers, there won't be any crash in Perl's signal handler as it's never
|
||||||
|
+# registered for those signals.
|
||||||
|
my $received_sigterm = 0;
|
||||||
|
$SIG{TERM} = sub { $received_sigterm += 1; note "received SIGTERM $received_sigterm"; };
|
||||||
|
+my $received_sigchld = 0;
|
||||||
|
+$SIG{CHLD} = sub { $received_sigchld += 1; note "received SIGCHLD $received_sigchld"; };
|
||||||
|
|
||||||
|
# initialize OpenCV via signalblocker and create_threads
|
||||||
|
{
|
||||||
|
@@ -75,6 +79,12 @@ waitpid $fork, 0;
|
||||||
|
note 'waiting for at least one signal to be handled' and sleep .2 until $received_sigterm >= 1 || ($timeout -= .2) < 0;
|
||||||
|
note "handled $received_sigterm TERM signals";
|
||||||
|
ok($received_sigterm > 0, "received SIGTERM $received_sigterm times; no crashes after at least 200 ms idling time");
|
||||||
|
+
|
||||||
|
+$received_sigchld = 0;
|
||||||
|
+# 0 here means WIFEXITED and WEXITSTATUS == 0
|
||||||
|
+cmp_ok(system("true"), '==', 0, 'system returns exit status');
|
||||||
|
+is($received_sigchld, 1, 'got SIGCHLD after system');
|
||||||
|
+
|
||||||
|
cmp_ok(thread_count, '<=', $last_thread_count, 'still no new threads after sending signals');
|
||||||
|
|
||||||
|
done_testing;
|
||||||
|
--
|
||||||
|
2.31.1
|
||||||
|
|
||||||
Loading…
Reference in new issue