211 lines
8.6 KiB
Diff
211 lines
8.6 KiB
Diff
From 4807d2d068ae9fc08b87121fc0a574394f8acc5b Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
|
|
Date: Sat, 4 Oct 2014 22:57:43 -0400
|
|
Subject: [PATCH] sd-event: be more careful when enabling/disabling signals
|
|
|
|
When a child event is disabled (in order to be freed) and there is no
|
|
SIGCHLD signal event, sd_event_source_set_enabled will disable SIGCHLD
|
|
even if there are other child events.
|
|
|
|
Also remove some unneeded signalfd updates.
|
|
|
|
https://bugs.freedesktop.org/show_bug.cgi?id=84659
|
|
|
|
Based-on-a-patch-by: Hristo Venev <mustrumr97@gmail.com>
|
|
---
|
|
src/libsystemd/sd-event/sd-event.c | 86 ++++++++++++++++++++++++++++----------
|
|
1 file changed, 63 insertions(+), 23 deletions(-)
|
|
|
|
diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
|
|
index 4c67ee87e1..c5f062b3e0 100644
|
|
--- a/src/libsystemd/sd-event/sd-event.c
|
|
+++ b/src/libsystemd/sd-event/sd-event.c
|
|
@@ -590,6 +590,14 @@ static struct clock_data* event_get_clock_data(sd_event *e, EventSourceType t) {
|
|
}
|
|
}
|
|
|
|
+static bool need_signal(sd_event *e, int signal) {
|
|
+ return (e->signal_sources && e->signal_sources[signal] &&
|
|
+ e->signal_sources[signal]->enabled != SD_EVENT_OFF)
|
|
+ ||
|
|
+ (signal == SIGCHLD &&
|
|
+ e->n_enabled_child_sources > 0);
|
|
+}
|
|
+
|
|
static void source_disconnect(sd_event_source *s) {
|
|
sd_event *event;
|
|
|
|
@@ -626,11 +634,13 @@ static void source_disconnect(sd_event_source *s) {
|
|
|
|
case SOURCE_SIGNAL:
|
|
if (s->signal.sig > 0) {
|
|
- if (s->signal.sig != SIGCHLD || s->event->n_enabled_child_sources == 0)
|
|
- assert_se(sigdelset(&s->event->sigset, s->signal.sig) == 0);
|
|
-
|
|
if (s->event->signal_sources)
|
|
s->event->signal_sources[s->signal.sig] = NULL;
|
|
+
|
|
+ /* If the signal was on and now it is off... */
|
|
+ if (s->enabled != SD_EVENT_OFF && !need_signal(s->event, s->signal.sig)) {
|
|
+ assert_se(sigdelset(&s->event->sigset, s->signal.sig) == 0);
|
|
+ }
|
|
}
|
|
|
|
break;
|
|
@@ -640,10 +650,12 @@ static void source_disconnect(sd_event_source *s) {
|
|
if (s->enabled != SD_EVENT_OFF) {
|
|
assert(s->event->n_enabled_child_sources > 0);
|
|
s->event->n_enabled_child_sources--;
|
|
- }
|
|
|
|
- if (!s->event->signal_sources || !s->event->signal_sources[SIGCHLD])
|
|
- assert_se(sigdelset(&s->event->sigset, SIGCHLD) == 0);
|
|
+ /* We know the signal was on, if it is off now... */
|
|
+ if (!need_signal(s->event, SIGCHLD)) {
|
|
+ assert_se(sigdelset(&s->event->sigset, SIGCHLD) == 0);
|
|
+ }
|
|
+ }
|
|
|
|
hashmap_remove(s->event->child_sources, INT_TO_PTR(s->child.pid));
|
|
}
|
|
@@ -963,6 +975,7 @@ _public_ int sd_event_add_signal(
|
|
sd_event_source *s;
|
|
sigset_t ss;
|
|
int r;
|
|
+ bool previous;
|
|
|
|
assert_return(e, -EINVAL);
|
|
assert_return(sig > 0, -EINVAL);
|
|
@@ -987,6 +1000,8 @@ _public_ int sd_event_add_signal(
|
|
} else if (e->signal_sources[sig])
|
|
return -EBUSY;
|
|
|
|
+ previous = need_signal(e, sig);
|
|
+
|
|
s = source_new(e, !ret, SOURCE_SIGNAL);
|
|
if (!s)
|
|
return -ENOMEM;
|
|
@@ -997,9 +1012,10 @@ _public_ int sd_event_add_signal(
|
|
s->enabled = SD_EVENT_ON;
|
|
|
|
e->signal_sources[sig] = s;
|
|
- assert_se(sigaddset(&e->sigset, sig) == 0);
|
|
|
|
- if (sig != SIGCHLD || e->n_enabled_child_sources == 0) {
|
|
+ if (!previous) {
|
|
+ assert_se(sigaddset(&e->sigset, sig) == 0);
|
|
+
|
|
r = event_update_signal_fd(e);
|
|
if (r < 0) {
|
|
source_free(s);
|
|
@@ -1023,6 +1039,7 @@ _public_ int sd_event_add_child(
|
|
|
|
sd_event_source *s;
|
|
int r;
|
|
+ bool previous;
|
|
|
|
assert_return(e, -EINVAL);
|
|
assert_return(pid > 1, -EINVAL);
|
|
@@ -1039,6 +1056,8 @@ _public_ int sd_event_add_child(
|
|
if (hashmap_contains(e->child_sources, INT_TO_PTR(pid)))
|
|
return -EBUSY;
|
|
|
|
+ previous = need_signal(e, SIGCHLD);
|
|
+
|
|
s = source_new(e, !ret, SOURCE_CHILD);
|
|
if (!s)
|
|
return -ENOMEM;
|
|
@@ -1057,9 +1076,9 @@ _public_ int sd_event_add_child(
|
|
|
|
e->n_enabled_child_sources ++;
|
|
|
|
- assert_se(sigaddset(&e->sigset, SIGCHLD) == 0);
|
|
+ if (!previous) {
|
|
+ assert_se(sigaddset(&e->sigset, SIGCHLD) == 0);
|
|
|
|
- if (!e->signal_sources || !e->signal_sources[SIGCHLD]) {
|
|
r = event_update_signal_fd(e);
|
|
if (r < 0) {
|
|
source_free(s);
|
|
@@ -1437,23 +1456,32 @@ _public_ int sd_event_source_set_enabled(sd_event_source *s, int m) {
|
|
}
|
|
|
|
case SOURCE_SIGNAL:
|
|
+ assert(need_signal(s->event, s->signal.sig));
|
|
+
|
|
s->enabled = m;
|
|
- if (s->signal.sig != SIGCHLD || s->event->n_enabled_child_sources == 0) {
|
|
+
|
|
+ if (!need_signal(s->event, s->signal.sig)) {
|
|
assert_se(sigdelset(&s->event->sigset, s->signal.sig) == 0);
|
|
- event_update_signal_fd(s->event);
|
|
+
|
|
+ (void) event_update_signal_fd(s->event);
|
|
+ /* If disabling failed, we might get a spurious event,
|
|
+ * but otherwise nothing bad should happen. */
|
|
}
|
|
|
|
break;
|
|
|
|
case SOURCE_CHILD:
|
|
+ assert(need_signal(s->event, SIGCHLD));
|
|
+
|
|
s->enabled = m;
|
|
|
|
assert(s->event->n_enabled_child_sources > 0);
|
|
s->event->n_enabled_child_sources--;
|
|
|
|
- if (!s->event->signal_sources || !s->event->signal_sources[SIGCHLD]) {
|
|
+ if (!need_signal(s->event, SIGCHLD)) {
|
|
assert_se(sigdelset(&s->event->sigset, SIGCHLD) == 0);
|
|
- event_update_signal_fd(s->event);
|
|
+
|
|
+ (void) event_update_signal_fd(s->event);
|
|
}
|
|
|
|
break;
|
|
@@ -1501,22 +1529,34 @@ _public_ int sd_event_source_set_enabled(sd_event_source *s, int m) {
|
|
}
|
|
|
|
case SOURCE_SIGNAL:
|
|
- s->enabled = m;
|
|
-
|
|
- if (s->signal.sig != SIGCHLD || s->event->n_enabled_child_sources == 0) {
|
|
+ /* Check status before enabling. */
|
|
+ if (!need_signal(s->event, s->signal.sig)) {
|
|
assert_se(sigaddset(&s->event->sigset, s->signal.sig) == 0);
|
|
- event_update_signal_fd(s->event);
|
|
+
|
|
+ r = event_update_signal_fd(s->event);
|
|
+ if (r < 0) {
|
|
+ s->enabled = SD_EVENT_OFF;
|
|
+ return r;
|
|
+ }
|
|
}
|
|
+
|
|
+ s->enabled = m;
|
|
break;
|
|
|
|
case SOURCE_CHILD:
|
|
+ /* Check status before enabling. */
|
|
if (s->enabled == SD_EVENT_OFF) {
|
|
- s->event->n_enabled_child_sources++;
|
|
-
|
|
- if (!s->event->signal_sources || !s->event->signal_sources[SIGCHLD]) {
|
|
- assert_se(sigaddset(&s->event->sigset, SIGCHLD) == 0);
|
|
- event_update_signal_fd(s->event);
|
|
+ if (!need_signal(s->event, SIGCHLD)) {
|
|
+ assert_se(sigaddset(&s->event->sigset, s->signal.sig) == 0);
|
|
+
|
|
+ r = event_update_signal_fd(s->event);
|
|
+ if (r < 0) {
|
|
+ s->enabled = SD_EVENT_OFF;
|
|
+ return r;
|
|
+ }
|
|
}
|
|
+
|
|
+ s->event->n_enabled_child_sources++;
|
|
}
|
|
|
|
s->enabled = m;
|