From 1af745d033678333752afcd8724f5d6351561b4e Mon Sep 17 00:00:00 2001 From: Dmitry Antipov Date: Tue, 25 Oct 2022 11:30:34 +0300 Subject: [PATCH] signal: new signal handling backend based on signalfd Linux-specific signal handling backend based on signalfd(2) system call, and public function event_base_get_signal_method() to obtain an underlying kernel signal handling mechanism. Signed-off-by: Dmitry Antipov --- CMakeLists.txt | 6 ++ Makefile.am | 3 + configure.ac | 3 +- epoll.c | 3 +- event-config.h.cmake | 3 + event.c | 7 ++ evmap.c | 5 +- evsignal-internal.h | 13 +++ include/event2/event.h | 16 ++++ poll.c | 3 +- select.c | 3 +- signal.c | 39 ++++---- signalfd.c | 213 +++++++++++++++++++++++++++++++++++++++++ test/regress.c | 12 ++- 14 files changed, 306 insertions(+), 23 deletions(-) create mode 100644 signalfd.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 94fe478000..8a86b1948a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -450,6 +450,7 @@ else() sys/wait.h sys/resource.h sys/timerfd.h + sys/signalfd.h netinet/in.h netinet/in6.h netinet/tcp.h @@ -583,6 +584,7 @@ list(APPEND CMAKE_EXTRA_INCLUDE_FILES ${EVENT_INCLUDES} stdio.h) CHECK_SYMBOLS_EXIST("${SYMBOLS_TO_CHECK}" "${CMAKE_EXTRA_INCLUDE_FILES}" "EVENT") unset(SYMBOLS_TO_CHECK) set(EVENT__HAVE_EPOLL ${EVENT__HAVE_EPOLL_CREATE}) +set(EVENT__HAVE_SIGNALFD ${EVENT__HAVE_SYS_SIGNALFD_H}) if(WIN32 AND NOT CYGWIN) set(EVENT__HAVE_WEPOLL 1) endif() @@ -904,6 +906,10 @@ if(EVENT__HAVE_EPOLL) list(APPEND SRC_CORE epoll.c) endif() +if(EVENT__HAVE_SIGNALFD) + list(APPEND SRC_CORE signalfd.c) +endif() + if(EVENT__HAVE_WEPOLL) list(APPEND SRC_CORE epoll.c diff --git a/Makefile.am b/Makefile.am index 2007bedaff..7e3acb0a72 100644 --- a/Makefile.am +++ b/Makefile.am @@ -229,6 +229,9 @@ endif if SIGNAL_SUPPORT SYS_SRC += signal.c endif +if SIGNALFD_SUPPORT +SYS_SRC += signalfd.c +endif BUILT_SOURCES += include/event2/event-config.h diff --git a/configure.ac b/configure.ac index 286b06c645..7a2c6d06da 100644 --- a/configure.ac +++ b/configure.ac @@ -181,7 +181,7 @@ LIBEVENT_OPENSSL LIBEVENT_MBEDTLS dnl Checks for header files. -AC_CHECK_HEADERS([arpa/inet.h fcntl.h ifaddrs.h mach/mach_time.h mach/mach.h netdb.h netinet/in.h netinet/in6.h netinet/tcp.h sys/un.h poll.h port.h stdarg.h stddef.h sys/devpoll.h sys/epoll.h sys/event.h sys/eventfd.h sys/ioctl.h sys/mman.h sys/param.h sys/queue.h sys/resource.h sys/select.h sys/sendfile.h sys/socket.h sys/stat.h sys/time.h sys/timerfd.h sys/uio.h sys/wait.h sys/random.h errno.h afunix.h]) +AC_CHECK_HEADERS([arpa/inet.h fcntl.h ifaddrs.h mach/mach_time.h mach/mach.h netdb.h netinet/in.h netinet/in6.h netinet/tcp.h sys/un.h poll.h port.h stdarg.h stddef.h sys/devpoll.h sys/epoll.h sys/event.h sys/eventfd.h sys/ioctl.h sys/mman.h sys/param.h sys/queue.h sys/resource.h sys/select.h sys/sendfile.h sys/socket.h sys/stat.h sys/time.h sys/timerfd.h sys/signalfd.h sys/uio.h sys/wait.h sys/random.h errno.h afunix.h]) case "${host_os}" in linux*) ;; @@ -543,6 +543,7 @@ if test "$bwin32" = "true"; then fi AM_CONDITIONAL(WEPOLL_BACKEND, [test "$havewepoll" = "yes"]) AM_CONDITIONAL(SIGNAL_SUPPORT, [test "$needsignal" = "yes"]) +AM_CONDITIONAL(SIGNALFD_SUPPORT, [test "$ac_cv_header_sys_signalfd_h" = "yes"]) AC_TYPE_PID_T AC_TYPE_SIZE_T diff --git a/epoll.c b/epoll.c index 9b4890772d..158a868d75 100644 --- a/epoll.c +++ b/epoll.c @@ -255,7 +255,8 @@ epoll_init(struct event_base *base) } #endif - evsig_init_(base); + if (sigfd_init_(base) < 0) + evsig_init_(base); return (epollop); } diff --git a/event-config.h.cmake b/event-config.h.cmake index a3cfafa164..08ac3dcb48 100644 --- a/event-config.h.cmake +++ b/event-config.h.cmake @@ -394,6 +394,9 @@ /* Define to 1 if you have the header file. */ #cmakedefine EVENT__HAVE_SYS_TIMERFD_H 1 +/* Define to 1 if you have the header file. */ +#cmakedefine EVENT__HAVE_SYS_SIGNALFD_H 1 + /* Define to 1 if you have the header file. */ #cmakedefine EVENT__HAVE_SYS_TIME_H 1 diff --git a/event.c b/event.c index a7a6b3f472..74678e1f5d 100644 --- a/event.c +++ b/event.c @@ -1860,6 +1860,13 @@ event_base_get_method(const struct event_base *base) return (base->evsel->name); } +const char * +event_base_get_signal_method(const struct event_base *base) +{ + EVUTIL_ASSERT(base); + return (base->evsigsel->name); +} + /** Callback: used to implement event_base_loopexit by telling the event_base * that it's time to exit its loop. */ static void diff --git a/evmap.c b/evmap.c index e4e35c6877..9084965460 100644 --- a/evmap.c +++ b/evmap.c @@ -465,7 +465,7 @@ evmap_signal_add_(struct event_base *base, int sig, struct event *ev) base->evsigsel->fdinfo_len); if (LIST_EMPTY(&ctx->events)) { - if (evsel->add(base, ev->ev_fd, 0, EV_SIGNAL, NULL) + if (evsel->add(base, ev->ev_fd, 0, EV_SIGNAL, ev) == -1) return (-1); } @@ -643,7 +643,8 @@ evmap_signal_reinit_iter_fn(struct event_base *base, int *result = arg; if (!LIST_EMPTY(&ctx->events)) { - if (evsel->add(base, signum, 0, EV_SIGNAL, NULL) == -1) + if (evsel->add(base, signum, 1, EV_SIGNAL, + LIST_FIRST(&ctx->events)) == -1) *result = -1; } return 0; diff --git a/evsignal-internal.h b/evsignal-internal.h index 5cff03b525..5ccf7d1536 100644 --- a/evsignal-internal.h +++ b/evsignal-internal.h @@ -45,6 +45,10 @@ struct evsig_info { int ev_signal_added; /* Count of the number of signals we're currently watching. */ int ev_n_signals_added; +#ifdef EVENT__HAVE_SYS_SIGNALFD_H + /* EV_READ events used to wakeup corresponding EV_SIGNAL ones. */ + struct event *ev_sigevent[NSIG]; +#endif /* EVENT__HAVE_SYS_SIGNALFD_H */ /* Array of previous signal handler objects before Libevent started * messing with them. Used to restore old signal handlers. */ @@ -56,8 +60,17 @@ struct evsig_info { /* Size of sh_old. */ int sh_old_max; }; + +#ifdef EVENT__HAVE_SYS_SIGNALFD_H +int sigfd_init_(struct event_base *); +#else /* no signalfd() */ +static inline int +sigfd_init_(struct event_base *base) { return -1; } +#endif /* have signalfd() */ + int evsig_init_(struct event_base *); void evsig_dealloc_(struct event_base *); +int evsig_ensure_saved_(struct evsig_info *, int); void evsig_set_base_(struct event_base *base); void evsig_free_globals_(void); diff --git a/include/event2/event.h b/include/event2/event.h index b52fd8464d..384a84178b 100644 --- a/include/event2/event.h +++ b/include/event2/event.h @@ -381,6 +381,18 @@ int event_base_dispatch(struct event_base *base); EVENT2_EXPORT_SYMBOL const char *event_base_get_method(const struct event_base *eb); +/** + Get the kernel signal handling mechanism used by Libevent. + + @param eb the event_base structure returned by event_base_new() + @return a string identifying the kernel signal handling mechanism, + which is "signal" for traditional UNIX signal handlers, + "kqueue_signal" for kqueue(2)-based method on *BSD and macOS, + and "signalfd_signal" for Linux-only signalfd(2)-based method. + */ +EVENT2_EXPORT_SYMBOL +const char *event_base_get_signal_method(const struct event_base *eb); + /** Gets all event notification mechanisms supported by Libevent. @@ -586,6 +598,10 @@ enum event_base_config_flag { epoll and if you do not have EVENT_BASE_FLAG_PRECISE_TIMER enabled. */ EVENT_BASE_FLAG_EPOLL_DISALLOW_TIMERFD = 0x40, + + /** Do not use signalfd(2) to handle signals even if supported. + */ + EVENT_BASE_FLAG_DISALLOW_SIGNALFD = 0x80, }; /** diff --git a/poll.c b/poll.c index c3c9aac52a..7991651916 100644 --- a/poll.c +++ b/poll.c @@ -103,7 +103,8 @@ poll_init(struct event_base *base) if (!(pollop = mm_calloc(1, sizeof(struct pollop)))) return (NULL); - evsig_init_(base); + if (sigfd_init_(base) < 0) + evsig_init_(base); evutil_weakrand_seed_(&base->weakrand_seed, 0); diff --git a/select.c b/select.c index b1db0e44b6..1277976dca 100644 --- a/select.c +++ b/select.c @@ -119,7 +119,8 @@ select_init(struct event_base *base) return (NULL); } - evsig_init_(base); + if (sigfd_init_(base) < 0) + evsig_init_(base); evutil_weakrand_seed_(&base->weakrand_seed, 0); diff --git a/signal.c b/signal.c index 551a454fe9..5cbafecb5e 100644 --- a/signal.c +++ b/signal.c @@ -208,25 +208,13 @@ evsig_init_(struct event_base *base) return 0; } -/* Helper: set the signal handler for evsignal to handler in base, so that - * we can restore the original handler when we clear the current one. */ +/* Helper: resize saved signal handler array up to the highest signal + number. A dynamic array is used to keep footprint on the low side. */ int -evsig_set_handler_(struct event_base *base, - int evsignal, void (__cdecl *handler)(int)) +evsig_ensure_saved_(struct evsig_info *sig, int evsignal) { -#ifdef EVENT__HAVE_SIGACTION - struct sigaction sa; -#else - ev_sighandler_t sh; -#endif - struct evsig_info *sig = &base->sig; - void *p; - - /* - * resize saved signal handler array up to the highest signal number. - * a dynamic array is used to keep footprint on the low side. - */ if (evsignal >= sig->sh_old_max) { + void *p; int new_max = evsignal + 1; event_debug(("%s: evsignal (%d) >= sh_old_max (%d), resizing", __func__, evsignal, sig->sh_old_max)); @@ -242,6 +230,25 @@ evsig_set_handler_(struct event_base *base, sig->sh_old_max = new_max; sig->sh_old = p; } + return 0; +} + +/* Helper: set the signal handler for evsignal to handler in base, so that + * we can restore the original handler when we clear the current one. */ +int +evsig_set_handler_(struct event_base *base, + int evsignal, void (__cdecl *handler)(int)) +{ +#ifdef EVENT__HAVE_SIGACTION + struct sigaction sa; +#else + ev_sighandler_t sh; +#endif + struct evsig_info *sig = &base->sig; + + /* ensure saved array is large enough */ + if (evsig_ensure_saved_(sig, evsignal) < 0) + return (-1); /* allocate space for previous handler out of dynamic array */ sig->sh_old[evsignal] = mm_malloc(sizeof *sig->sh_old[evsignal]); diff --git a/signalfd.c b/signalfd.c new file mode 100644 index 0000000000..376a04d539 --- /dev/null +++ b/signalfd.c @@ -0,0 +1,213 @@ +/* + * Signal handling backend based on signalfd(2) system call + * Written by Dmitry Antipov 2022 + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "event2/event-config.h" + +#include +#include + +#include "event2/event.h" +#include "event-internal.h" +#include "evmap-internal.h" +#include "evsignal-internal.h" +#include "evthread-internal.h" + +static int sigfd_add(struct event_base *, evutil_socket_t, short, short, void *); +static int sigfd_del(struct event_base *, evutil_socket_t, short, short, void *); + +static const struct eventop sigfdops = { + "signalfd_signal", + NULL, + sigfd_add, + sigfd_del, + NULL, + NULL, + 0, 0, 0 +}; + +static void +sigfd_cb(evutil_socket_t fd, short what, void *arg) +{ + struct signalfd_siginfo fdsi; + struct event_base *base = arg; + ssize_t ret = read(fd, &fdsi, sizeof(fdsi)); + + EVUTIL_ASSERT(ret == sizeof(fdsi)); + EVUTIL_ASSERT(fdsi.ssi_signo > 0 && fdsi.ssi_signo < NSIG); + EVUTIL_ASSERT(base->sig.ev_sigevent[fdsi.ssi_signo] != NULL); + + EVBASE_ACQUIRE_LOCK(base, th_base_lock); + evmap_signal_active_(base, fdsi.ssi_signo, 1); + EVBASE_RELEASE_LOCK(base, th_base_lock); +} + +static void +sigfd_free_sigevent(struct event_base *base, int signo) +{ + int ret; + struct event* sigev = base->sig.ev_sigevent[signo]; + + EVUTIL_ASSERT(sigev != NULL); + event_del_nolock_(sigev, EVENT_DEL_AUTOBLOCK); + ret = close(sigev->ev_fd); + EVUTIL_ASSERT(ret == 0); + mm_free(sigev); + base->sig.ev_sigevent[signo] = NULL; +} + +static int +sigfd_add(struct event_base *base, int signo, short old, short events, void *p) +{ + int sigfd; + sigset_t mask; + struct event* sigev; + struct evsig_info *sig = &base->sig; + + /* EV_SIGNAL event passed from evmap_signal_add_() when setting + up and from evmap_signal_reinit_iter_fn() during reinit. */ + EVUTIL_ASSERT(p != NULL); + + EVUTIL_ASSERT(signo > 0 && signo < NSIG); + sigev = base->sig.ev_sigevent[signo]; + + if (sigev != NULL) { + if (old) { + /* We're performing reinit after fork(). This is + required at least for epoll(2)-based backend + because if the process uses fork(2) to create + a child process, then the child will be able + to read(2) signals that are sent to it using the + signalfd(2) file descriptor, but epoll_wait(2) + will not indicate that the signalfd file + descriptor is ready. */ + sigfd_free_sigevent(base, signo); + } else { + /* We have an active signal fd + for this signal already. */ + return 0; + } + } + + /* Save previous handler just like evsig_set_handler_() does. */ + if (evsig_ensure_saved_(sig, signo) < 0) + return -1; + + sig->sh_old[signo] = mm_malloc(sizeof *sig->sh_old[signo]); + if (sig->sh_old[signo] == NULL) { + event_warn("malloc() failed"); + return -1; + } + + if (sigaction(signo, NULL, sig->sh_old[signo]) == -1) { + event_warn("sigaction() failed"); + mm_free(sig->sh_old[signo]); + sig->sh_old[signo] = NULL; + return -1; + } + + /* Block the signal from being handled according to its default + disposition so it may be received via the descriptor. */ + sigemptyset(&mask); + sigaddset(&mask, signo); + if (sigprocmask(SIG_BLOCK, &mask, NULL)) { + event_warn("sigprocmask() failed"); + return -1; + } + + sigfd = signalfd(-1, &mask, SFD_NONBLOCK | SFD_CLOEXEC); + if (sigfd < 0) { + event_warn("signalfd() failed"); + goto unblock; + } + + /* EV_READ event used to wakeup corresponding EV_SIGNAL ones. */ + sigev = event_new(base, sigfd, EV_READ | EV_PERSIST, sigfd_cb, base); + if (!sigev) + goto close_fd; + + /* This was blindly copied from evsig_init_(). */ + sigev->ev_flags |= EVLIST_INTERNAL; + event_priority_set(sigev, 0); + + if (event_add_nolock_(sigev, NULL, 0) < 0) + goto free_ev; + + base->sig.ev_sigevent[signo] = sigev; + return 0; +free_ev: + mm_free(sigev); +close_fd: + close(sigfd); +unblock: + sigprocmask(SIG_UNBLOCK, &mask, NULL); + return -1; +} + +static int +sigfd_del(struct event_base *base, int signo, short old, short events, void *p) +{ + sigset_t mask; + struct event *sigev; + struct evsig_info *sig = &base->sig; + + EVUTIL_ASSERT(signo > 0 && signo < NSIG); + sigev = base->sig.ev_sigevent[signo]; + EVUTIL_ASSERT(sigev != NULL); + + sigemptyset(&mask); + sigaddset(&mask, signo); + if (sigprocmask(SIG_UNBLOCK, &mask, NULL)) { + event_warn("sigprocmask() failed"); + return -1; + } + + /* Restore previous handler, if any. */ + if (signo < sig->sh_old_max) { + struct sigaction *sa = sig->sh_old[signo]; + if (sa) { + if (sigaction(signo, sa, NULL) == -1) { + event_warn("sigaction() failed"); + return -1; + } + mm_free(sa); + sig->sh_old[signo] = NULL; + } + } + + sigfd_free_sigevent(base, signo); + return 0; +} + +int sigfd_init_(struct event_base *base) +{ + EVUTIL_ASSERT(base != NULL); + if ((base->flags & EVENT_BASE_FLAG_DISALLOW_SIGNALFD) || + getenv("EVENT_DISALLOW_SIGNALFD")) + return -1; + base->evsigsel = &sigfdops; + return 0; +} diff --git a/test/regress.c b/test/regress.c index 148e22911f..63c935e5d5 100644 --- a/test/regress.c +++ b/test/regress.c @@ -1260,6 +1260,9 @@ test_signal_pipeloss(void) * make two bases to catch signals, use both of them. this only works * for event mechanisms that use our signal pipe trick. kqueue handles * signals internally, and all interested kqueues get all the signals. + * This is not expected to work with signalfd - having more than one + * descriptor in attempt to accept the same signal (or intersecting sets + * of signals) is not the thing signalfd() was designed for. */ static void test_signal_switchbase(void) @@ -1267,9 +1270,16 @@ test_signal_switchbase(void) struct event ev1, ev2; struct event_base *base1, *base2; int is_kqueue; - test_ok = 0; base1 = event_init(); base2 = event_init(); + + test_ok = 1; + if (!strcmp(event_base_get_signal_method(base1), "signalfd_signal") || + !strcmp(event_base_get_signal_method(base2), "signalfd_signal")) { + tt_skip(); + } + test_ok = 0; + is_kqueue = !strcmp(event_get_method(),"kqueue"); evsignal_set(&ev1, SIGUSR1, signal_cb, &ev1); evsignal_set(&ev2, SIGUSR1, signal_cb, &ev2);