Problem in tevent_signal

[Volker Lendecke]

On Sat, Jan 28, 2012 at 09:13:00PM +0100, Volker Lendecke wrote:
> Destroying a tevent_signal event from within its handler
> isn't supported right now. See the following code sequence
> from tevent_signal.c:
> 
>         se->handler(ev, se, i, count, NULL, se->private_data);
> #ifdef SA_RESETHAND
>         if (se->sa_flags & SA_RESETHAND) {
>                 talloc_free(se);
>         }
> #endif
> 
> The signal handler might be gone after it was executed.
> 
> The trick used in tevent_timed is probably not really
> appropriate, we might run the same event more than once.
> 
> Working on it, maybe someone else has a bright idea how to
> solve this.

Not the most elegant fix, but it seems to fix the issue for
me. Needs commenting I think. And maybe we should re-use the
object by doing talloc_steal.

Comments?

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
>From 68326aeb26d936a5d7384135cdf9b9fdcbb09ae6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 28 Jan 2012 22:18:00 +0100
Subject: [PATCH] tevent: Fix deleting signal events from within themselves

---
 lib/tevent/tevent_signal.c |   28 ++++++++++++++++++++++++++--
 1 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/lib/tevent/tevent_signal.c b/lib/tevent/tevent_signal.c
index fabe72c..e642920 100644
--- a/lib/tevent/tevent_signal.c
+++ b/lib/tevent/tevent_signal.c
@@ -307,6 +307,15 @@ struct tevent_signal *tevent_common_add_signal(struct tevent_context *ev,
 	return se;
 }
 
+struct tevent_se_exists {
+	struct tevent_se_exists **myself;
+};
+
+static int tevent_se_exists_destructor(struct tevent_se_exists *s)
+{
+	*s->myself = NULL;
+	return 0;
+}
 
 /*
   check if a signal is pending
@@ -335,6 +344,16 @@ int tevent_common_check_signal(struct tevent_context *ev)
 		}
 		for (sl=sig_state->sig_handlers[i];sl;sl=next) {
 			struct tevent_signal *se = sl->se;
+			struct tevent_se_exists *exists;
+
+			exists = talloc(se, struct tevent_se_exists);
+			if (exists == NULL) {
+				continue;
+			}
+			exists->myself = &exists;
+			talloc_set_destructor(
+				exists, tevent_se_exists_destructor);
+
 			next = sl->next;
 #ifdef SA_SIGINFO
 			if (se->sa_flags & SA_SIGINFO) {
@@ -352,21 +371,26 @@ int tevent_common_check_signal(struct tevent_context *ev)
 					se->handler(ev, se, i, 1,
 						    (void*)&sig_state->sig_info[i][ofs], 
 						    se->private_data);
+					if (!exists) {
+						break;
+					}
 				}
 #ifdef SA_RESETHAND
-				if (se->sa_flags & SA_RESETHAND) {
+				if (exists && (se->sa_flags & SA_RESETHAND)) {
 					talloc_free(se);
 				}
 #endif
+				talloc_free(exists);
 				continue;
 			}
 #endif
 			se->handler(ev, se, i, count, NULL, se->private_data);
 #ifdef SA_RESETHAND
-			if (se->sa_flags & SA_RESETHAND) {
+			if (exists && (se->sa_flags & SA_RESETHAND)) {
 				talloc_free(se);
 			}
 #endif
+			talloc_free(exists);
 		}
 
 #ifdef SA_SIGINFO
-- 
1.7.5.4