From d5edd64257e6fd45cbef71b456e9bc8c5b6177b0 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 25 Feb 2021 22:35:13 +0100 Subject: [PATCH] If a signal handler runs during `select()` loop in `clixon_event_loop()` and unless the signal handler sets clixon_exit, the select will be restarted. --- CHANGELOG.md | 3 +++ apps/backend/backend_main.c | 22 +++++++++++++++++++++- lib/src/clixon_event.c | 15 ++++++++++++--- lib/src/clixon_sig.c | 2 +- test/test_restconf_rpc.sh | 3 +++ 5 files changed, 40 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 24334c3c..fb1b0f5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -112,6 +112,9 @@ Developers may need to change their code ### Minor changes +* If a signal handler runs during `select()` loop in `clixon_event_loop()` and unless the signal handler sets clixon_exit, the select will be restarted. + * Existing behavior for SIGTERM/SIGINT to exit is maintained + * This was for supporting SIGCHLD of forked restconf that crashes or being killed externally. * Look for symbols in plugins using `dlsym(RTLD_DEFAULT)` instead of `dlsym(NULL)` for more portable use * Thanks jdl@netgate.com * Added support for the following XPATH functions: diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index c030e196..4aa1d7f1 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -158,6 +159,21 @@ backend_sig_term(int arg) clicon_exit_set(); /* checked in clixon_event_loop() */ } +/*! wait for killed child + * primary use in case restconf daemon forked using process-control API + * This may cause EINTR in eg select() in clixon_event_loop() which will be ignored + */ +static void +backend_sig_child(int arg) +{ + int status; + int pid; + + clicon_debug(1, "%s", __FUNCTION__); + if ((pid = waitpid(-1, &status, 0)) != -1 && WIFEXITED(status)){ + } +} + /*! Create backend server socket and register callback * @param[in] h Clicon handle * @retval s Server socket file descriptor (see socket(2)) @@ -996,7 +1012,11 @@ main(int argc, clicon_err(OE_DAEMON, errno, "Setting signal"); goto done; } - + /* This is in case restconf daemon forked using process-control API */ + if (set_signal(SIGCHLD, backend_sig_child, NULL) < 0){ + clicon_err(OE_DAEMON, errno, "Setting signal"); + goto done; + } /* Initialize server socket and save it to handle */ if ((ss = backend_server_socket(h)) < 0) goto done; diff --git a/lib/src/clixon_event.c b/lib/src/clixon_event.c index 455d15e6..1e573e62 100644 --- a/lib/src/clixon_event.c +++ b/lib/src/clixon_event.c @@ -55,6 +55,7 @@ #include "clixon_queue.h" #include "clixon_log.h" #include "clixon_err.h" +#include "clixon_sig.h" #include "clixon_event.h" /* @@ -199,7 +200,7 @@ clixon_event_unreg_fd(int s, * Note also that the callback is not periodic, you need to make a new * registration for each period, see example above. * Note also that the first argument to fn is a dummy, just to get the same - * signatute as for file-descriptor callbacks. + * signature as for file-descriptor callbacks. * @see clixon_event_reg_fd * @see clixon_event_unreg_timeout */ @@ -319,10 +320,18 @@ clixon_event_loop(void) if (clicon_exit_get()) break; if (n == -1) { + /* Signals either set clixon_exit, then the function returns 0 + * Typically for set_signal() of SIGTERM,SIGINT, etc + * Other signals are ignored, and the select is rerun, eg SIGCHLD + */ if (errno == EINTR){ clicon_debug(1, "%s select: %s", __FUNCTION__, strerror(errno)); - clicon_err(OE_EVENTS, errno, "select"); - retval = 0; + if (clicon_exit_get()){ + clicon_err(OE_EVENTS, errno, "select"); + retval = 0; + } + else + continue; } else clicon_err(OE_EVENTS, errno, "select"); diff --git a/lib/src/clixon_sig.c b/lib/src/clixon_sig.c index d54ba1db..16fbe661 100644 --- a/lib/src/clixon_sig.c +++ b/lib/src/clixon_sig.c @@ -68,7 +68,7 @@ set_signal(int signo, snew.sa_handler = handler; sigemptyset(&snew.sa_mask); - snew.sa_flags = 0; + snew.sa_flags = SA_RESTART; if (sigaction(signo, &snew, &sold) < 0){ clicon_err(OE_UNIX, errno, "sigaction"); return -1; diff --git a/test/test_restconf_rpc.sh b/test/test_restconf_rpc.sh index c047b3e2..42813b9b 100755 --- a/test/test_restconf_rpc.sh +++ b/test/test_restconf_rpc.sh @@ -136,6 +136,9 @@ if [ -z "$ps" ]; then err "A restconf running" fi +new "try restconf rpc" +expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/operations/clixon-lib:process-control -d '{"clixon-lib:input":{"name":"restconf","operation":"status"}}')" 0 "HTTP/1.1 200 OK" '{"clixon-lib:output":{"pid":' + new "stop restconf RPC" pid=$(testrpc stop 0) if [ $? -ne 0 ]; then echo "$pid";exit -1; fi