From ad0c237bc0b2edd4f1dc0e82c6cffbde71bad78b Mon Sep 17 00:00:00 2001 From: Nicholas Marriott <@nicm> Date: Sun, 27 Dec 2015 01:43:37 +0300 Subject: [PATCH 1/5] event_reinit: always re-init signal's socketpair Before this patch event_reinit() only closes the signal socketpair fds and recreates them if signals have been added, but this is wrong, since socketpair fds created on backend init, and if we will not re-create them bad things in child/parent signal handling will happens (and indeed this is what happens for non-reinit backends like select). Fixes: #307 --- event.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/event.c b/event.c index def17665..082d292f 100644 --- a/event.c +++ b/event.c @@ -969,13 +969,13 @@ event_reinit(struct event_base *base) event_del_nolock_(&base->sig.ev_signal, EVENT_DEL_AUTOBLOCK); event_debug_unassign(&base->sig.ev_signal); memset(&base->sig.ev_signal, 0, sizeof(base->sig.ev_signal)); - if (base->sig.ev_signal_pair[0] != -1) - EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[0]); - if (base->sig.ev_signal_pair[1] != -1) - EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[1]); had_signal_added = 1; base->sig.ev_signal_added = 0; } + if (base->sig.ev_signal_pair[0] != -1) + EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[0]); + if (base->sig.ev_signal_pair[1] != -1) + EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[1]); if (base->th_notify_fn != NULL) { was_notifiable = 1; base->th_notify_fn = NULL; @@ -1024,8 +1024,7 @@ event_reinit(struct event_base *base) if (evmap_reinit_(base) < 0) res = -1; } else { - if (had_signal_added) - res = evsig_init_(base); + res = evsig_init_(base); } /* If we were notifiable before, and nothing just exploded, become From 88640aa1ca1d974dd4b301f9c496f417921af69b Mon Sep 17 00:00:00 2001 From: Nicholas Marriott <@nicm> Date: Sun, 27 Dec 2015 02:15:03 +0300 Subject: [PATCH 2/5] event_reinit: make signals works after fork() without evsig_add() event_reinit() removes the event, but only evsig_add puts it back. So any signals set up before event_reinit will be ignored until another signal is added. Fixes: #307 --- event.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/event.c b/event.c index 082d292f..503003e2 100644 --- a/event.c +++ b/event.c @@ -1025,6 +1025,11 @@ event_reinit(struct event_base *base) res = -1; } else { res = evsig_init_(base); + if (res == 0 && had_signal_added) { + res = event_add_nolock_(&base->sig.ev_signal, NULL, 0); + if (res == 0) + base->sig.ev_signal_added = 1; + } } /* If we were notifiable before, and nothing just exploded, become From 088d8b39f9f72cd83941bcb9bc261fabfd432eb2 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sun, 27 Dec 2015 02:31:03 +0300 Subject: [PATCH 3/5] test/regress: main/fork: rewrite assertions by just removing event in callback Instead of assigning some variable value (got_child), and schedule exit from loop from that callback, just remove event for that signal, and event loop will exit automatically when there will be no events. --- test/regress.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/test/regress.c b/test/regress.c index 4d170820..d200ae35 100644 --- a/test/regress.c +++ b/test/regress.c @@ -817,28 +817,21 @@ end: } #ifndef _WIN32 -static void signal_cb(evutil_socket_t fd, short event, void *arg); #define current_base event_global_current_base_ extern struct event_base *current_base; static void -child_signal_cb(evutil_socket_t fd, short event, void *arg) +fork_signal_cb(evutil_socket_t fd, short events, void *arg) { - struct timeval tv; - int *pint = arg; - - *pint = 1; - - tv.tv_usec = 500000; - tv.tv_sec = 0; - event_loopexit(&tv); + event_del(arg); } + static void test_fork(void) { - int status, got_sigchld = 0; + int status; struct event ev, sig_ev; pid_t pid; @@ -855,7 +848,7 @@ test_fork(void) if (event_add(&ev, NULL) == -1) exit(1); - evsignal_set(&sig_ev, SIGCHLD, child_signal_cb, &got_sigchld); + evsignal_set(&sig_ev, SIGCHLD, fork_signal_cb, &sig_ev); evsignal_add(&sig_ev, NULL); event_base_assert_ok_(current_base); @@ -917,11 +910,6 @@ test_fork(void) event_dispatch(); - if (!got_sigchld) { - fprintf(stdout, "FAILED (sigchld)\n"); - exit(1); - } - evsignal_del(&sig_ev); end: From b075b81cd347aaf3c170940819d6857e0f67e75e Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sun, 27 Dec 2015 09:26:54 +0300 Subject: [PATCH 4/5] test/regress: cover signals after fork() + event_reinit() Regression-for: ad0c237 ("event_reinit: always re-init signal's socketpair") --- test/regress.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/regress.c b/test/regress.c index d200ae35..7d785904 100644 --- a/test/regress.c +++ b/test/regress.c @@ -832,7 +832,7 @@ static void test_fork(void) { int status; - struct event ev, sig_ev; + struct event ev, sig_ev, usr_ev; pid_t pid; setup_test("After fork: "); @@ -867,6 +867,10 @@ test_fork(void) evsignal_del(&sig_ev); + evsignal_set(&usr_ev, SIGUSR1, fork_signal_cb, &usr_ev); + evsignal_add(&usr_ev, NULL); + raise(SIGUSR1); + called = 0; event_dispatch(); @@ -908,6 +912,10 @@ test_fork(void) shutdown(pair[0], SHUT_WR); + evsignal_set(&usr_ev, SIGUSR1, fork_signal_cb, &usr_ev); + evsignal_add(&usr_ev, NULL); + raise(SIGUSR1); + event_dispatch(); evsignal_del(&sig_ev); From ceddc607ca3ab1fd695e300e5c5a545f11752d47 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sun, 27 Dec 2015 02:48:40 +0300 Subject: [PATCH 5/5] test/regress: cover existing signal callbacks and fork() + event_reinit() Regression-for: 88640aa ("event_reinit: make signals works after fork() without evsig_add()") --- test/regress.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/regress.c b/test/regress.c index 7d785904..ae548696 100644 --- a/test/regress.c +++ b/test/regress.c @@ -832,7 +832,7 @@ static void test_fork(void) { int status; - struct event ev, sig_ev, usr_ev; + struct event ev, sig_ev, usr_ev, existing_ev; pid_t pid; setup_test("After fork: "); @@ -851,6 +851,9 @@ test_fork(void) evsignal_set(&sig_ev, SIGCHLD, fork_signal_cb, &sig_ev); evsignal_add(&sig_ev, NULL); + evsignal_set(&existing_ev, SIGUSR2, fork_signal_cb, &existing_ev); + evsignal_add(&existing_ev, NULL); + event_base_assert_ok_(current_base); TT_BLATHER(("Before fork")); if ((pid = regress_fork()) == 0) { @@ -870,6 +873,7 @@ test_fork(void) evsignal_set(&usr_ev, SIGUSR1, fork_signal_cb, &usr_ev); evsignal_add(&usr_ev, NULL); raise(SIGUSR1); + raise(SIGUSR2); called = 0; @@ -915,6 +919,7 @@ test_fork(void) evsignal_set(&usr_ev, SIGUSR1, fork_signal_cb, &usr_ev); evsignal_add(&usr_ev, NULL); raise(SIGUSR1); + raise(SIGUSR2); event_dispatch();