from trunk: support multiple events listening on the same signal; make signals regular events that go on the same event queue

svn:r902

ChangeLog

@@ -6,6 +6,7 @@ Changes in 1.4.6-stable:
  o Reject negative Content-Length headers; anonymous bug report
  o Detect CLOCK_MONOTONIC at runtime for evdns; anonymous bug report	
  o Fix a bug where deleting signals with the kqueue backend would cause subsequent adds to fail
+ o Support multiple events listening on the same signal; make signals regular events that go on the same event queue.
 
 Changes in 1.4.5-stable:
  o Fix connection keep-alive behavior for HTTP/1.0

event.c

@@ -187,7 +187,6 @@ event_base_new(void)
 	
 	min_heap_ctor(&base->timeheap);
 	TAILQ_INIT(&base->eventqueue);
-	TAILQ_INIT(&base->sig.signalqueue);
 	base->sig.ev_signal_pair[0] = -1;
 	base->sig.ev_signal_pair[1] = -1;
 	
@@ -455,7 +454,7 @@ event_base_loop(struct event_base *base, int flags)
 	struct timeval *tv_p;
 	int res, done;
 
-	if(!TAILQ_EMPTY(&base->sig.signalqueue))
+	if (&base->sig.ev_signal_added)
 		evsignal_base = base;
 	done = 0;
 	while (!done) {
@@ -667,13 +666,11 @@ event_pending(struct event *ev, short event, struct timeval *tv)
 	int flags = 0;
 
 	if (ev->ev_flags & EVLIST_INSERTED)
-		flags |= (ev->ev_events & (EV_READ|EV_WRITE));
+		flags |= (ev->ev_events & (EV_READ|EV_WRITE|EV_SIGNAL));
 	if (ev->ev_flags & EVLIST_ACTIVE)
 		flags |= ev->ev_res;
 	if (ev->ev_flags & EVLIST_TIMEOUT)
 		flags |= EV_TIMEOUT;
-	if (ev->ev_flags & EVLIST_SIGNAL)
-		flags |= EV_SIGNAL;
 
 	event &= (EV_TIMEOUT|EV_READ|EV_WRITE|EV_SIGNAL);
 
@@ -741,19 +738,12 @@ event_add(struct event *ev, const struct timeval *tv)
 		event_queue_insert(base, ev, EVLIST_TIMEOUT);
 	}
 
-	if ((ev->ev_events & (EV_READ|EV_WRITE)) &&
+	if ((ev->ev_events & (EV_READ|EV_WRITE|EV_SIGNAL)) &&
 	    !(ev->ev_flags & (EVLIST_INSERTED|EVLIST_ACTIVE))) {
 		int res = evsel->add(evbase, ev);
 		if (res != -1)
 			event_queue_insert(base, ev, EVLIST_INSERTED);
 
-		return (res);
-	} else if ((ev->ev_events & EV_SIGNAL) &&
-	    !(ev->ev_flags & EVLIST_SIGNAL)) {
-		int res = evsel->add(evbase, ev);
-		if (res != -1)
-			event_queue_insert(base, ev, EVLIST_SIGNAL);
-
 		return (res);
 	}
 
@@ -795,9 +785,6 @@ event_del(struct event *ev)
 	if (ev->ev_flags & EVLIST_INSERTED) {
 		event_queue_remove(base, ev, EVLIST_INSERTED);
 		return (evsel->del(evbase, ev));
-	} else if (ev->ev_flags & EVLIST_SIGNAL) {
-		event_queue_remove(base, ev, EVLIST_SIGNAL);
-		return (evsel->del(evbase, ev));
 	}
 
 	return (0);
@@ -934,9 +921,6 @@ event_queue_remove(struct event_base *base, struct event *ev, int queue)
 	case EVLIST_TIMEOUT:
 		min_heap_erase(&base->timeheap, ev);
 		break;
-	case EVLIST_SIGNAL:
-		TAILQ_REMOVE(&base->sig.signalqueue, ev, ev_signal_next);
-		break;
 	default:
 		event_errx(1, "%s: unknown queue %x", __func__, queue);
 	}
@@ -971,9 +955,6 @@ event_queue_insert(struct event_base *base, struct event *ev, int queue)
 		min_heap_push(&base->timeheap, ev);
 		break;
 	}
-	case EVLIST_SIGNAL:
-		TAILQ_INSERT_TAIL(&base->sig.signalqueue, ev, ev_signal_next);
-		break;
 	default:
 		event_errx(1, "%s: unknown queue %x", __func__, queue);
 	}

evsignal.h

@@ -30,11 +30,11 @@
 typedef void (*ev_sighandler_t)(int);
 
 struct evsignal_info {
-	struct event_list signalqueue;
 	struct event ev_signal;
 	int ev_signal_pair[2];
 	int ev_signal_added;
 	volatile sig_atomic_t evsignal_caught;
+	struct event_list evsigevents[NSIG];
 	sig_atomic_t evsigcaught[NSIG];
 #ifdef HAVE_SIGACTION
 	struct sigaction **sh_old;

kqueue.c

@@ -44,6 +44,7 @@
 #include <string.h>
 #include <unistd.h>
 #include <errno.h>
+#include <assert.h>
 #ifdef HAVE_INTTYPES_H
 #include <inttypes.h>
 #endif
@@ -70,6 +71,7 @@ struct kqop {
 	struct kevent *changes;
 	int nchanges;
 	struct kevent *events;
+	struct event_list evsigevents[NSIG];
 	int nevents;
 	int kq;
 	pid_t pid;
@@ -95,7 +97,7 @@ const struct eventop kqops = {
 static void *
 kq_init(struct event_base *base)
 {
-	int kq;
+	int i, kq;
 	struct kqop *kqueueop;
 
 	/* Disable kqueue when this environment variable is set */
@@ -131,6 +133,11 @@ kq_init(struct event_base *base)
 	}
 	kqueueop->nevents = NEVENT;
 
+	/* we need to keep track of multiple events per signal */
+	for (i = 0; i < NSIG; ++i) {
+		TAILQ_INIT(&kqueueop->evsigevents[i]);
+	}
+
 	/* Check for Mac OS X kqueue bug. */
 	kqueueop->changes[0].ident = -1;
 	kqueueop->changes[0].filter = EVFILT_READ;
@@ -259,8 +266,6 @@ kq_dispatch(struct event_base *base, void *arg, struct timeval *tv)
 			return (-1);
 		}
 
-		ev = (struct event *)events[i].udata;
-
 		if (events[i].filter == EVFILT_READ) {
 			which |= EV_READ;
 		} else if (events[i].filter == EVFILT_WRITE) {
@@ -272,11 +277,20 @@ kq_dispatch(struct event_base *base, void *arg, struct timeval *tv)
 		if (!which)
 			continue;
 
-		if (!(ev->ev_events & EV_PERSIST))
+		if (events[i].filter == EVFILT_SIGNAL) {
-			ev->ev_flags &= ~EVLIST_X_KQINKERNEL;
+			struct event_list *head =
+			    (struct event_list *)events[i].udata;
+			TAILQ_FOREACH(ev, head, ev_signal_next) {
+				event_active(ev, which, events[i].data);
+			}
+		} else {
+			ev = (struct event *)events[i].udata;
 
-		event_active(ev, which,
+			if (!(ev->ev_events & EV_PERSIST))
-		    ev->ev_events & EV_SIGNAL ? events[i].data : 1);
+				ev->ev_flags &= ~EVLIST_X_KQINKERNEL;
+
+			event_active(ev, which, 1);
+		}
 	}
 
 	return (0);
@@ -291,25 +305,30 @@ kq_add(void *arg, struct event *ev)
 
 	if (ev->ev_events & EV_SIGNAL) {
 		int nsignal = EVENT_SIGNAL(ev);
-                struct timespec timeout = { 0, 0 };
 
- 		memset(&kev, 0, sizeof(kev));
+		assert(nsignal >= 0 && nsignal < NSIG);
-		kev.ident = nsignal;
+		if (TAILQ_EMPTY(&kqop->evsigevents[nsignal])) {
-		kev.filter = EVFILT_SIGNAL;
+			struct timespec timeout = { 0, 0 };
-		kev.flags = EV_ADD;
+			
-		if (!(ev->ev_events & EV_PERSIST))
+			memset(&kev, 0, sizeof(kev));
-			kev.flags |= EV_ONESHOT;
+			kev.ident = nsignal;
-		kev.udata = PTR_TO_UDATA(ev);
+			kev.filter = EVFILT_SIGNAL;
-
+			kev.flags = EV_ADD;
-		/* Be ready for the signal if it is sent any time between
+			kev.udata = PTR_TO_UDATA(&kqop->evsigevents[nsignal]);
-		 * now and the next call to kq_dispatch. */
+			
-                if (kevent(kqop->kq, &kev, 1, NULL, 0, &timeout) == -1)
+			/* Be ready for the signal if it is sent any
-                	return (-1);
+			 * time between now and the next call to
-
+			 * kq_dispatch. */
-		if (_evsignal_set_handler(ev->ev_base, nsignal,
+			if (kevent(kqop->kq, &kev, 1, NULL, 0, &timeout) == -1)
-					  kq_sighandler) == -1)
+				return (-1);
-			return (-1);
+			
+			if (_evsignal_set_handler(ev->ev_base, nsignal,
+				kq_sighandler) == -1)
+				return (-1);
+		}
 
+		TAILQ_INSERT_TAIL(&kqop->evsigevents[nsignal], ev,
+		    ev_signal_next);
 		ev->ev_flags |= EVLIST_X_KQINKERNEL;
 		return (0);
 	}
@@ -364,18 +383,24 @@ kq_del(void *arg, struct event *ev)
 		int nsignal = EVENT_SIGNAL(ev);
 		struct timespec timeout = { 0, 0 };
 
- 		memset(&kev, 0, sizeof(kev));
+		assert(nsignal >= 0 && nsignal < NSIG);
-		kev.ident = nsignal;
+		TAILQ_REMOVE(&kqop->evsigevents[nsignal], ev, ev_signal_next);
-		kev.filter = EVFILT_SIGNAL;
+		if (TAILQ_EMPTY(&kqop->evsigevents[nsignal])) {
-		kev.flags = EV_DELETE;
+			memset(&kev, 0, sizeof(kev));
+			kev.ident = nsignal;
+			kev.filter = EVFILT_SIGNAL;
+			kev.flags = EV_DELETE;
 		
-		/* Because we insert signal events immediately, we need to
+			/* Because we insert signal events
-		 * delete them immediately, too */
+			 * immediately, we need to delete them
-                if (kevent(kqop->kq, &kev, 1, NULL, 0, &timeout) == -1)
+			 * immediately, too */
-                	return (-1);
+			if (kevent(kqop->kq, &kev, 1, NULL, 0, &timeout) == -1)
+				return (-1);
 
-		if (_evsignal_restore_handler(ev->ev_base, nsignal) == -1)
+			if (_evsignal_restore_handler(ev->ev_base,
-			return (-1);
+				nsignal) == -1)
+				return (-1);
+		}
 
 		ev->ev_flags &= ~EVLIST_X_KQINKERNEL;
 		return (0);

signal.c

@@ -95,12 +95,15 @@ evsignal_cb(int fd, short what, void *arg)
 void
 evsignal_init(struct event_base *base)
 {
+	int i;
+
 	/* 
 	 * Our signal handler is going to write to one end of the socket
 	 * pair to wake up our event loop.  The event loop then scans for
 	 * signals that got delivered.
 	 */
-	if (evutil_socketpair(AF_UNIX, SOCK_STREAM, 0, base->sig.ev_signal_pair) == -1)
+	if (evutil_socketpair(
+		    AF_UNIX, SOCK_STREAM, 0, base->sig.ev_signal_pair) == -1)
 		event_err(1, "%s: socketpair", __func__);
 
 	FD_CLOSEONEXEC(base->sig.ev_signal_pair[0]);
@@ -109,6 +112,9 @@ evsignal_init(struct event_base *base)
 	base->sig.sh_old_max = 0;
 	base->sig.evsignal_caught = 0;
 	memset(&base->sig.evsigcaught, 0, sizeof(sig_atomic_t)*NSIG);
+	/* initialize the queues for all events */
+	for (i = 0; i < NSIG; ++i)
+		TAILQ_INIT(&base->sig.evsigevents[i]);
 
         evutil_make_socket_nonblocking(base->sig.ev_signal_pair[0]);
 
@@ -189,19 +195,26 @@ evsignal_add(struct event *ev)
 	if (ev->ev_events & (EV_READ|EV_WRITE))
 		event_errx(1, "%s: EV_SIGNAL incompatible use", __func__);
 	evsignal = EVENT_SIGNAL(ev);
+	assert(evsignal >= 0 & evsignal < NSIG);
+	if (TAILQ_EMPTY(&sig->evsigevents[evsignal])) {
+		event_debug(("%s: %p: changing signal handler", __func__, ev));
+		if (_evsignal_set_handler(
+			    base, evsignal, evsignal_handler) == -1)
+			return (-1);
 
-	event_debug(("%s: %p: changing signal handler", __func__, ev));
+		/* catch signals if they happen quickly */
-	if (_evsignal_set_handler(base, evsignal, evsignal_handler) == -1)
+		evsignal_base = base;
-		return (-1);
 
-	/* catch signals if they happen quickly */
+		if (!sig->ev_signal_added) {
-	evsignal_base = base;
+			if (event_add(&sig->ev_signal, NULL))
-
+				return (-1);
-	if (!sig->ev_signal_added) {
+			sig->ev_signal_added = 1;
-		sig->ev_signal_added = 1;
+		}
-		event_add(&sig->ev_signal, NULL);
 	}
 
+	/* multiple events may listen to the same signal */
+	TAILQ_INSERT_TAIL(&sig->evsigevents[evsignal], ev, ev_signal_next);
+
 	return (0);
 }
 
@@ -238,8 +251,21 @@ _evsignal_restore_handler(struct event_base *base, int evsignal)
 int
 evsignal_del(struct event *ev)
 {
+	struct event_base *base = ev->ev_base;
+	struct evsignal_info *sig = &base->sig;
+	int evsignal = EVENT_SIGNAL(ev);
+
+	assert(evsignal >= 0 & evsignal < NSIG);
+
+	/* multiple events may listen to the same signal */
+	TAILQ_REMOVE(&sig->evsigevents[evsignal], ev, ev_signal_next);
+
+	if (!TAILQ_EMPTY(&sig->evsigevents[evsignal]))
+		return (0);
+
 	event_debug(("%s: %p: restoring signal handler", __func__, ev));
-	return _evsignal_restore_handler(ev->ev_base, EVENT_SIGNAL(ev));
+
+	return (_evsignal_restore_handler(ev->ev_base, EVENT_SIGNAL(ev)));
 }
 
 static void
@@ -247,7 +273,7 @@ evsignal_handler(int sig)
 {
 	int save_errno = errno;
 
-	if(evsignal_base == NULL) {
+	if (evsignal_base == NULL) {
 		event_warn(
 			"%s: received signal %d, but have no base configured",
 			__func__, sig);
@@ -269,29 +295,39 @@ evsignal_handler(int sig)
 void
 evsignal_process(struct event_base *base)
 {
-	struct event *ev;
+	struct evsignal_info *sig = &base->sig;
+	struct event *ev, *next_ev;
 	sig_atomic_t ncalls;
-
+	int i;
+	
 	base->sig.evsignal_caught = 0;
-	TAILQ_FOREACH(ev, &base->sig.signalqueue, ev_signal_next) {
+	for (i = 1; i < NSIG; ++i) {
-		ncalls = base->sig.evsigcaught[EVENT_SIGNAL(ev)];
+		ncalls = sig->evsigcaught[i];
-		if (ncalls) {
+		if (ncalls == 0)
+			continue;
+
+		for (ev = TAILQ_FIRST(&sig->evsigevents[i]);
+		    ev != NULL; ev = next_ev) {
+			next_ev = TAILQ_NEXT(ev, ev_signal_next);
 			if (!(ev->ev_events & EV_PERSIST))
 				event_del(ev);
 			event_active(ev, EV_SIGNAL, ncalls);
-			base->sig.evsigcaught[EVENT_SIGNAL(ev)] = 0;
 		}
+
+		sig->evsigcaught[i] = 0;
 	}
 }
 
 void
 evsignal_dealloc(struct event_base *base)
 {
-	if(base->sig.ev_signal_added) {
+	int i = 0;
+	if (base->sig.ev_signal_added) {
 		event_del(&base->sig.ev_signal);
 		base->sig.ev_signal_added = 0;
 	}
-	assert(TAILQ_EMPTY(&base->sig.signalqueue));
+	for (i = 0; i < NSIG; ++i)
+		assert(TAILQ_EMPTY(&base->sig.evsigevents[0]));
 
 	EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[0]);
 	base->sig.ev_signal_pair[0] = -1;

test/regress.c

@@ -534,6 +534,36 @@ test_simplesignal(void)
 	cleanup_test();
 }
 
+static void
+test_multiplesignal(void)
+{
+	struct event ev_one, ev_two;
+	struct itimerval itv;
+
+	setup_test("Multiple signal: ");
+
+	signal_set(&ev_one, SIGALRM, signal_cb, &ev_one);
+	signal_add(&ev_one, NULL);
+
+	signal_set(&ev_two, SIGALRM, signal_cb, &ev_two);
+	signal_add(&ev_two, NULL);
+
+	memset(&itv, 0, sizeof(itv));
+	itv.it_value.tv_sec = 1;
+	if (setitimer(ITIMER_REAL, &itv, NULL) == -1)
+		goto skip_simplesignal;
+
+	event_dispatch();
+
+ skip_simplesignal:
+	if (signal_del(&ev_one) == -1)
+		test_ok = 0;
+	if (signal_del(&ev_two) == -1)
+		test_ok = 0;
+
+	cleanup_test();
+}
+
 static void
 test_immediatesignal(void)
 {
@@ -1506,6 +1536,7 @@ main (int argc, char **argv)
 	test_simpletimeout();
 #ifndef WIN32
 	test_simplesignal();
+	test_multiplesignal();
 	test_immediatesignal();
 #endif
 	test_loopexit();