From 594ab34f1dfc73db85e8f95ec51892cadecaa76c Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 10 Jul 2023 10:40:49 +0200 Subject: [PATCH] Disable signalfd by default signalfd may behave differently to sigaction/signal, so to avoid breaking libevent users (like [1], [2]) disable it by default. [1]: https://github.com/tmux/tmux/pull/3621 [2]: https://github.com/tmux/tmux/pull/3626 Also signalfd is not that perfect: - you need to SIG_BLOCK the signal before - blocked signals are not reset on exec - blocked signals are allowed to coalesce - so in case of multiple signals sent you may get the signal only once (ok for most of the signals, but may be a problem for SIGCHLD, though you may call waitpid() in a loop or use pidfd) - and also one implementation problem - sigprocmask is unspecified in a multithreaded process Refs: - https://lwn.net/Articles/415684/ - https://ldpreload.com/blog/signalfd-is-useless Refs: https://github.com/libevent/libevent/issues/1460 Refs: #1342 (cc @dmantipov) --- CMakeLists.txt | 1 + include/event2/event.h | 6 ++++-- signalfd.c | 4 ++-- test/include.am | 2 ++ test/test.sh | 11 +++++++++-- 5 files changed, 18 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cd41d16e57..9c402ec0c1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1509,6 +1509,7 @@ if (NOT EVENT__DISABLE_TESTS) else() add_backend_test(${BACKEND} "${BACKEND_ENV_VARS}") endif() + add_backend_test(signalfd_${BACKEND} "${BACKEND_ENV_VARS};EVENT_USE_SIGNALFD=1") endforeach() # diff --git a/include/event2/event.h b/include/event2/event.h index 384a84178b..9b971edf1d 100644 --- a/include/event2/event.h +++ b/include/event2/event.h @@ -599,9 +599,11 @@ enum event_base_config_flag { */ EVENT_BASE_FLAG_EPOLL_DISALLOW_TIMERFD = 0x40, - /** Do not use signalfd(2) to handle signals even if supported. + /** Use signalfd(2) to handle signals over sigaction/signal. + * + * But note, that in some edge cases signalfd() may works differently. */ - EVENT_BASE_FLAG_DISALLOW_SIGNALFD = 0x80, + EVENT_BASE_FLAG_USE_SIGNALFD = 0x80, }; /** diff --git a/signalfd.c b/signalfd.c index 376a04d539..ed31014e5f 100644 --- a/signalfd.c +++ b/signalfd.c @@ -205,8 +205,8 @@ sigfd_del(struct event_base *base, int signo, short old, short events, void *p) int sigfd_init_(struct event_base *base) { EVUTIL_ASSERT(base != NULL); - if ((base->flags & EVENT_BASE_FLAG_DISALLOW_SIGNALFD) || - getenv("EVENT_DISALLOW_SIGNALFD")) + if (!(base->flags & EVENT_BASE_FLAG_USE_SIGNALFD) && + !getenv("EVENT_USE_SIGNALFD")) return -1; base->evsigsel = &sigfdops; return 0; diff --git a/test/include.am b/test/include.am index e061c937b7..9b50759da7 100644 --- a/test/include.am +++ b/test/include.am @@ -80,6 +80,8 @@ test_runner_changelist: $(top_srcdir)/test/test.sh $(top_srcdir)/test/test.sh -b "" -c test_runner_timerfd_changelist: $(top_srcdir)/test/test.sh $(top_srcdir)/test/test.sh -b "" -T +test_runner_timerfd_changelist: $(top_srcdir)/test/test.sh + $(top_srcdir)/test/test.sh -b "" -S DISTCLEANFILES += test/regress.gen.c test/regress.gen.h diff --git a/test/test.sh b/test/test.sh index dfdd2bf098..79362888c5 100755 --- a/test/test.sh +++ b/test/test.sh @@ -50,6 +50,7 @@ setup () { done unset EVENT_EPOLL_USE_CHANGELIST unset EVENT_PRECISE_TIMER + unset EVENT_USE_SIGNALFD } announce () { @@ -138,10 +139,12 @@ do_test() { EVENT_EPOLL_USE_CHANGELIST=yes; export EVENT_EPOLL_USE_CHANGELIST elif test "$2" = "(timerfd)" ; then EVENT_PRECISE_TIMER=1; export EVENT_PRECISE_TIMER + elif test "$2" = "(signalfd)" ; then + EVENT_USE_SIGNALFD=1; export EVENT_USE_SIGNALFD elif test "$2" = "(timerfd+changelist)" ; then EVENT_EPOLL_USE_CHANGELIST=yes; export EVENT_EPOLL_USE_CHANGELIST EVENT_PRECISE_TIMER=1; export EVENT_PRECISE_TIMER - fi + fi run_tests } @@ -153,6 +156,7 @@ usage() -t - run timerfd test -c - run changelist test -T - run timerfd+changelist test + -S - run signalfd test EOL } main() @@ -161,13 +165,15 @@ main() timerfd=0 changelist=0 timerfd_changelist=0 + signalfd=0 - while getopts "b:tcT" c; do + while getopts "b:tcTS" c; do case "$c" in b) backends="$OPTARG";; t) timerfd=1;; c) changelist=1;; T) timerfd_changelist=1;; + S) signalfd=1;; ?*) usage && exit 1;; esac done @@ -179,6 +185,7 @@ main() [ $timerfd_changelist -eq 0 ] || do_test EPOLL "(timerfd+changelist)" for i in $backends; do do_test $i + [ $signalfd -eq 0 ] || do_test $i "(signalfd)" done if test "$FAILED" = "yes"; then