diff --git a/contrib/nghttpx-logrotate b/contrib/nghttpx-logrotate index f1002bd6..c715cede 100644 --- a/contrib/nghttpx-logrotate +++ b/contrib/nghttpx-logrotate @@ -6,6 +6,6 @@ delaycompress notifempty postrotate - killall -USR1 nghttpx 2> /dev/null || true + [ -s /var/run/nghttpx.pid ] && kill -USR1 `cat /var/run/nghttpx.pid` 2> /dev/null || true endscript } diff --git a/doc/nghttpx.h2r b/doc/nghttpx.h2r index ed97fad2..aa9ab106 100644 --- a/doc/nghttpx.h2r +++ b/doc/nghttpx.h2r @@ -49,6 +49,10 @@ SIGUSR2 the latter. The former is called master process, and the latter is called worker process. The above signal must be sent to the master process. If the worker process receives one of them, it is ignored. + This behaviour of worker process may change in the future release. + In other words, in the future release, worker process may terminate + upon the reception of these signals. Therefore these signals should + not be sent to the worker process. SERVER PUSH ----------- diff --git a/integration-tests/server_tester.go b/integration-tests/server_tester.go index 9f41e620..8b6c4faa 100644 --- a/integration-tests/server_tester.go +++ b/integration-tests/server_tester.go @@ -21,6 +21,7 @@ import ( "sort" "strconv" "strings" + "syscall" "testing" "time" ) @@ -203,10 +204,20 @@ func (st *serverTester) Close() { st.conn.Close() } if st.cmd != nil { - st.cmd.Process.Kill() - st.cmd.Wait() - // workaround to unreliable Process.Signal() - time.Sleep(150 * time.Millisecond) + done := make(chan struct{}) + go func() { + st.cmd.Wait() + done <- struct{}{} + }() + + st.cmd.Process.Signal(syscall.SIGQUIT) + + select { + case <-done: + case <-time.After(10 * time.Second): + st.cmd.Process.Kill() + <-done + } } if st.ts != nil { st.ts.Close() diff --git a/src/Makefile.am b/src/Makefile.am index 358e9d8d..3635b633 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -127,6 +127,7 @@ NGHTTPX_SRCS = \ shrpx_memcached_result.h \ shrpx_worker_process.cc shrpx_worker_process.h \ shrpx_process.h \ + shrpx_signal.cc shrpx_signal.h \ buffer.h memchunk.h template.h if HAVE_SPDYLAY diff --git a/src/shrpx.cc b/src/shrpx.cc index 3798312a..57d546e4 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -81,6 +81,7 @@ #include "shrpx_http2_session.h" #include "shrpx_worker_process.h" #include "shrpx_process.h" +#include "shrpx_signal.h" #include "util.h" #include "app_helper.h" #include "ssl.h" @@ -210,26 +211,55 @@ void save_pid() { namespace { void exec_binary(SignalServer *ssv) { + int rv; + sigset_t oldset; + LOG(NOTICE) << "Executing new binary"; - auto pid = fork(); - - if (pid == -1) { + rv = shrpx_signal_block_all(&oldset); + if (rv != 0) { auto error = errno; - LOG(ERROR) << "fork() failed errno=" << error; + LOG(ERROR) << "Blocking all signals failed: " << strerror(error); + return; } + auto pid = fork(); + if (pid != 0) { + if (pid == -1) { + auto error = errno; + LOG(ERROR) << "fork() failed errno=" << error; + } + + rv = shrpx_signal_set(&oldset); + + if (rv != 0) { + auto error = errno; + LOG(FATAL) << "Restoring signal mask failed: " << strerror(error); + + exit(EXIT_FAILURE); + } + return; } + shrpx_signal_unset_master_proc_ign_handler(); + + rv = shrpx_signal_unblock_all(); + if (rv != 0) { + auto error = errno; + LOG(ERROR) << "Unblocking all signals failed: " << strerror(error); + + exit(EXIT_FAILURE); + } + auto exec_path = util::get_exec_path(get_config()->argc, get_config()->argv, get_config()->cwd); if (!exec_path) { LOG(ERROR) << "Could not resolve the executable path"; - return; + exit(EXIT_FAILURE); } auto argv = make_unique(get_config()->argc + 1); @@ -306,7 +336,7 @@ void exec_binary(SignalServer *ssv) { if (execve(argv[0], argv.get(), envp.get()) == -1) { auto error = errno; LOG(ERROR) << "execve failed: errno=" << error; - _Exit(EXIT_FAILURE); + exit(EXIT_FAILURE); } } } // namespace @@ -647,20 +677,59 @@ void close_env_fd(std::initializer_list envnames) { namespace { pid_t fork_worker_process(SignalServer *ssv) { int rv; - auto pid = fork(); + sigset_t oldset; + + rv = shrpx_signal_block_all(&oldset); + if (rv != 0) { + auto error = errno; + LOG(ERROR) << "Blocking all signals failed: " << strerror(error); - if (pid == -1) { return -1; } + auto pid = fork(); + if (pid == 0) { + ev_loop_fork(EV_DEFAULT); + + shrpx_signal_set_worker_proc_ign_handler(); + + rv = shrpx_signal_unblock_all(); + if (rv != 0) { + auto error = errno; + LOG(FATAL) << "Unblocking all signals failed: " << strerror(error); + + exit(EXIT_FAILURE); + } + close(ssv->ipc_fd[1]); WorkerProcessConfig wpconf{ssv->ipc_fd[0], ssv->server_fd, ssv->server_fd6}; rv = worker_process_event_loop(&wpconf); if (rv != 0) { - LOG(ERROR) << "Worker process returned error"; + LOG(FATAL) << "Worker process returned error"; + + exit(EXIT_FAILURE); } - return 0; + + exit(EXIT_SUCCESS); + } + + // parent process + if (pid == -1) { + auto error = errno; + LOG(ERROR) << "Could not spawn worker process: " << strerror(error); + } + + rv = shrpx_signal_set(&oldset); + if (rv != 0) { + auto error = errno; + LOG(FATAL) << "Restoring signal mask failed: " << strerror(error); + + exit(EXIT_FAILURE); + } + + if (pid == -1) { + return -1; } close(ssv->ipc_fd[0]); @@ -675,6 +744,8 @@ namespace { int event_loop() { int rv; + shrpx_signal_set_master_proc_ign_handler(); + if (get_config()->daemon) { if (call_daemon() == -1) { auto error = errno; @@ -710,8 +781,6 @@ int event_loop() { util::make_socket_closeonexec(fd); } - auto loop = EV_DEFAULT; - if (get_config()->host_unix) { close_env_fd({ENV_LISTENER4_FD, ENV_LISTENER6_FD}); auto fd = create_unix_domain_server_socket(); @@ -746,19 +815,17 @@ int event_loop() { ssv.server_fd6 = fd6; } + auto loop = EV_DEFAULT; + auto pid = fork_worker_process(&ssv); - switch (pid) { - case -1: + if (pid == -1) { return -1; - case 0: - // worker process (child) - return 0; } ssv.worker_process_pid = pid; - auto signals = + constexpr auto signals = std::array{{REOPEN_LOG_SIGNAL, EXEC_BINARY_SIGNAL, GRACEFUL_SHUTDOWN_SIGNAL, SIGINT, SIGTERM}}; auto sigevs = std::array(); @@ -2444,10 +2511,6 @@ int main(int argc, char **argv) { reset_timer(); } - struct sigaction act {}; - act.sa_handler = SIG_IGN; - sigaction(SIGPIPE, &act, nullptr); - if (event_loop() != 0) { return -1; } diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index 76cb51c4..5610b06b 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -43,6 +43,7 @@ #include "shrpx_downstream_connection.h" #include "shrpx_accept_handler.h" #include "shrpx_memcached_dispatcher.h" +#include "shrpx_signal.h" #include "util.h" #include "template.h" @@ -432,30 +433,62 @@ int ConnectionHandler::start_ocsp_update(const char *cert_file) { } }); - auto pid = fork(); - if (pid == -1) { + sigset_t oldset; + + rv = shrpx_signal_block_all(&oldset); + if (rv != 0) { auto error = errno; - LOG(WARN) << "Could not execute ocsp query command for " << cert_file - << ": " << argv[0] << ", fork() failed, errno=" << error; + LOG(ERROR) << "Blocking all signals failed: " << strerror(error); + return -1; } + auto pid = fork(); + if (pid == 0) { // child process + shrpx_signal_unset_worker_proc_ign_handler(); + + rv = shrpx_signal_unblock_all(); + if (rv != 0) { + auto error = errno; + LOG(FATAL) << "Unblocking all signals failed: " << strerror(error); + + exit(EXIT_FAILURE); + } + dup2(pfd[1], 1); close(pfd[0]); rv = execve(argv[0], argv, envp); if (rv == -1) { auto error = errno; - LOG(WARN) << "Could not execute ocsp query command: " << argv[0] - << ", execve() faild, errno=" << error; - _Exit(EXIT_FAILURE); + LOG(ERROR) << "Could not execute ocsp query command: " << argv[0] + << ", execve() faild, errno=" << error; + exit(EXIT_FAILURE); } // unreachable } // parent process + if (pid == -1) { + auto error = errno; + LOG(ERROR) << "Could not execute ocsp query command for " << cert_file + << ": " << argv[0] << ", fork() failed, errno=" << error; + } + + rv = shrpx_signal_set(&oldset); + if (rv != 0) { + auto error = errno; + LOG(FATAL) << "Restoring all signals failed: " << strerror(error); + + exit(EXIT_FAILURE); + } + + if (pid == -1) { + return -1; + } + close(pfd[1]); pfd[1] = -1; diff --git a/src/shrpx_process.h b/src/shrpx_process.h index 47fc5b89..d35461b9 100644 --- a/src/shrpx_process.h +++ b/src/shrpx_process.h @@ -27,17 +27,11 @@ #include "shrpx.h" -#include - namespace shrpx { constexpr uint8_t SHRPX_IPC_REOPEN_LOG = 1; constexpr uint8_t SHRPX_IPC_GRACEFUL_SHUTDOWN = 2; -constexpr int REOPEN_LOG_SIGNAL = SIGUSR1; -constexpr int EXEC_BINARY_SIGNAL = SIGUSR2; -constexpr int GRACEFUL_SHUTDOWN_SIGNAL = SIGQUIT; - } // namespace shrpx #endif // SHRPX_PROCESS_H diff --git a/src/shrpx_signal.cc b/src/shrpx_signal.cc new file mode 100644 index 00000000..f689d4fa --- /dev/null +++ b/src/shrpx_signal.cc @@ -0,0 +1,129 @@ +/* + * nghttp2 - HTTP/2 C Library + * + * Copyright (c) 2015 Tatsuhiro Tsujikawa + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +#include "shrpx_signal.h" + +#include + +#include "template.h" + +using namespace nghttp2; + +namespace shrpx { + +int shrpx_signal_block_all(sigset_t *oldset) { + sigset_t newset; + int rv; + + sigfillset(&newset); + +#ifndef NOTHREADS + rv = pthread_sigmask(SIG_SETMASK, &newset, oldset); + + if (rv != 0) { + errno = rv; + return -1; + } + + return 0; +#else // NOTHREADS + return sigprocmask(SIG_SETMASK, &newset, &oldset); +#endif // NOTHREADS +} + +int shrpx_signal_unblock_all() { + sigset_t newset; + int rv; + + sigemptyset(&newset); + +#ifndef NOTHREADS + rv = pthread_sigmask(SIG_SETMASK, &newset, nullptr); + + if (rv != 0) { + errno = rv; + return -1; + } + + return 0; +#else // NOTHREADS + return sigprocmask(SIG_SETMASK, &newset, nullptr); +#endif // NOTHREADS +} + +int shrpx_signal_set(sigset_t *set) { + int rv; + +#ifndef NOTHREADS + rv = pthread_sigmask(SIG_SETMASK, set, nullptr); + + if (rv != 0) { + errno = rv; + return -1; + } + + return 0; +#else // NOTHREADS + return sigprocmask(SIG_SETMASK, set, nullptr); +#endif // NOTHREADS +} + +namespace { +template +void signal_set_handler(void (*handler)(int), Signals &&sigs) { + struct sigaction act {}; + act.sa_handler = handler; + sigemptyset(&act.sa_mask); + for (auto sig : sigs) { + sigaction(sig, &act, nullptr); + } +} +} // namespace + +namespace { +constexpr auto master_proc_ign_signals = std::array{{SIGPIPE}}; +} // namespace + +namespace { +constexpr auto worker_proc_ign_signals = std::array{ + {REOPEN_LOG_SIGNAL, EXEC_BINARY_SIGNAL, GRACEFUL_SHUTDOWN_SIGNAL, SIGPIPE}}; +} // namespace + +void shrpx_signal_set_master_proc_ign_handler() { + signal_set_handler(SIG_IGN, master_proc_ign_signals); +} + +void shrpx_signal_unset_master_proc_ign_handler() { + signal_set_handler(SIG_DFL, master_proc_ign_signals); +} + +void shrpx_signal_set_worker_proc_ign_handler() { + signal_set_handler(SIG_IGN, worker_proc_ign_signals); +} + +void shrpx_signal_unset_worker_proc_ign_handler() { + signal_set_handler(SIG_DFL, worker_proc_ign_signals); +} + +} // namespace shrpx diff --git a/src/shrpx_signal.h b/src/shrpx_signal.h new file mode 100644 index 00000000..dcedd792 --- /dev/null +++ b/src/shrpx_signal.h @@ -0,0 +1,59 @@ +/* + * nghttp2 - HTTP/2 C Library + * + * Copyright (c) 2015 Tatsuhiro Tsujikawa + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +#ifndef SHRPX_SIGNAL_H +#define SHRPX_SIGNAL_H + +#include "shrpx.h" + +#include + +namespace shrpx { + +constexpr int REOPEN_LOG_SIGNAL = SIGUSR1; +constexpr int EXEC_BINARY_SIGNAL = SIGUSR2; +constexpr int GRACEFUL_SHUTDOWN_SIGNAL = SIGQUIT; + +// Blocks all signals. The previous signal mask is stored into +// |oldset| if it is not nullptr. This function returns 0 if it +// succeeds, or -1. The errno will indicate the error. +int shrpx_signal_block_all(sigset_t *oldset); + +// Unblocks all signals. This function returns 0 if it succeeds, or +// -1. The errno will indicate the error. +int shrpx_signal_unblock_all(); + +// Sets signal mask |set|. This function returns 0 if it succeeds, or +// -1. The errno will indicate the error. +int shrpx_signal_set(sigset_t *set); + +void shrpx_signal_set_master_proc_ign_handler(); +void shrpx_signal_unset_master_proc_ign_handler(); + +void shrpx_signal_set_worker_proc_ign_handler(); +void shrpx_signal_unset_worker_proc_ign_handler(); + +} // namespace shrpx + +#endif // SHRPX_SIGNAL_H diff --git a/src/shrpx_worker_process.cc b/src/shrpx_worker_process.cc index 76411c02..bda182dc 100644 --- a/src/shrpx_worker_process.cc +++ b/src/shrpx_worker_process.cc @@ -422,23 +422,36 @@ int worker_process_event_loop(WorkerProcessConfig *wpconf) { int rv; - // It is good to ignore these control signals since user may use - // "killall .. nghttpx". We don't want catch this signal in worker - // process. - struct sigaction act {}; - act.sa_handler = SIG_IGN; - sigaction(REOPEN_LOG_SIGNAL, &act, nullptr); - sigaction(EXEC_BINARY_SIGNAL, &act, nullptr); - sigaction(GRACEFUL_SHUTDOWN_SIGNAL, &act, nullptr); - if (get_config()->num_worker == 1) { rv = conn_handler.create_single_worker(); + if (rv != 0) { + return -1; + } } else { - rv = conn_handler.create_worker_thread(get_config()->num_worker); - } +#ifndef NOTHREADS + sigset_t set; + sigemptyset(&set); + sigaddset(&set, SIGCHLD); - if (rv != 0) { - return -1; + rv = pthread_sigmask(SIG_BLOCK, &set, nullptr); + if (rv != 0) { + LOG(ERROR) << "Blocking SIGCHLD failed: " << strerror(rv); + return -1; + } +#endif // !NOTHREADS + + rv = conn_handler.create_worker_thread(get_config()->num_worker); + if (rv != 0) { + return -1; + } + +#ifndef NOTHREADS + rv = pthread_sigmask(SIG_UNBLOCK, &set, nullptr); + if (rv != 0) { + LOG(ERROR) << "Unblocking SIGCHLD failed: " << strerror(rv); + return -1; + } +#endif // !NOTHREADS } drop_privileges();