From a54cda22abc4e235cc4e67f1588b74e5c33912e5 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 30 Jul 2016 23:58:19 +0900 Subject: [PATCH 01/11] nghttpx: Do creation of InheritedAddr in a dedicated function for reuse --- src/shrpx.cc | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 86e789bc..499fc7af 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -660,11 +660,8 @@ int create_tcp_server_socket(UpstreamAddr &faddr, } // namespace namespace { -int create_acceptor_socket() { +std::vector get_inherited_addr_from_env() { int rv; - - auto &listenerconf = mod_config()->conn.listener; - std::vector iaddrs; { @@ -806,6 +803,26 @@ int create_acceptor_socket() { } } + return iaddrs; +} +} // namespace + +namespace { +void closeUnusedInheritedAddr(const std::vector &iaddrs) { + for (auto &ia : iaddrs) { + if (ia.used) { + continue; + } + + close(ia.fd); + } +} +} // namespace + +namespace { +int create_acceptor_socket(std::vector &iaddrs) { + auto &listenerconf = mod_config()->conn.listener; + for (auto &addr : listenerconf.addrs) { if (addr.host_unix) { if (create_unix_domain_server_socket(addr, iaddrs) != 0) { @@ -829,14 +846,6 @@ int create_acceptor_socket() { } } - for (auto &ia : iaddrs) { - if (ia.used) { - continue; - } - - close(ia.fd); - } - return 0; } } // namespace @@ -957,10 +966,14 @@ int event_loop() { util::make_socket_closeonexec(fd); } - if (create_acceptor_socket() != 0) { + auto iaddrs = get_inherited_addr_from_env(); + + if (create_acceptor_socket(iaddrs) != 0) { return -1; } + closeUnusedInheritedAddr(iaddrs); + auto loop = ev_default_loop(get_config()->ev_loop_flags); auto pid = fork_worker_process(&ssv); From 1214f9e23b41079e573939e850472c99de51c153 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 31 Jul 2016 15:57:41 +0900 Subject: [PATCH 02/11] nghttpx: Reload configuration with SIGHUP This commit implements configuration reloading with SIGHUP. There are rough edges left: * Rename SignalServer with more meaningful name, say, WorkerProcess. * We should introduce global configuration object which is not affected by configuration reloading. It should hold cmdcfgs, argc, argv, and last worker PID. * We should close the listener file descriptor when some operation was failed after that. --- src/shrpx.cc | 533 +++++++++++++++++++++++++++++++++----------- src/shrpx_config.cc | 43 +++- src/shrpx_config.h | 16 +- src/shrpx_signal.cc | 5 +- src/shrpx_signal.h | 1 + 5 files changed, 458 insertions(+), 140 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 499fc7af..d6c9426d 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -130,22 +130,138 @@ constexpr auto ENV_ACCEPT_PREFIX = StringRef::from_lit("NGHTTPX_ACCEPT_"); #endif #endif +struct InheritedAddr { + // IP address if TCP socket. Otherwise, UNIX domain socket path. + ImmutableString host; + uint16_t port; + // true if UNIX domain socket path + bool host_unix; + int fd; + bool used; +}; + +namespace { +std::random_device rd; +} // namespace + +namespace { +// This contains all options given in command-line. Make it static so +// that we can use it in reloading. +std::vector> cmdcfgs; +} // namespace + +namespace { +void signal_cb(struct ev_loop *loop, ev_signal *w, int revents); +} // namespace + +namespace { +void worker_process_child_cb(struct ev_loop *loop, ev_child *w, int revents); +} // namespace + struct SignalServer { - SignalServer() : ipc_fd{{-1, -1}}, worker_process_pid(-1) {} + SignalServer(struct ev_loop *loop, pid_t worker_pid, int ipc_fd) + : loop(loop), worker_pid(worker_pid), ipc_fd(ipc_fd) { + ev_signal_init(&reopen_log_signalev, signal_cb, REOPEN_LOG_SIGNAL); + reopen_log_signalev.data = this; + ev_signal_start(loop, &reopen_log_signalev); + + ev_signal_init(&exec_binary_signalev, signal_cb, EXEC_BINARY_SIGNAL); + exec_binary_signalev.data = this; + ev_signal_start(loop, &exec_binary_signalev); + + ev_signal_init(&graceful_shutdown_signalev, signal_cb, + GRACEFUL_SHUTDOWN_SIGNAL); + graceful_shutdown_signalev.data = this; + ev_signal_start(loop, &graceful_shutdown_signalev); + + ev_signal_init(&reload_signalev, signal_cb, RELOAD_SIGNAL); + reload_signalev.data = this; + ev_signal_start(loop, &reload_signalev); + + ev_child_init(&worker_process_childev, worker_process_child_cb, worker_pid, + 0); + worker_process_childev.data = this; + ev_child_start(loop, &worker_process_childev); + } + ~SignalServer() { - if (ipc_fd[0] != -1) { - close(ipc_fd[0]); - } - if (ipc_fd[1] != -1) { - shutdown(ipc_fd[1], SHUT_WR); - close(ipc_fd[1]); + shutdown_signal_watchers(); + + ev_child_stop(loop, &worker_process_childev); + + if (ipc_fd != -1) { + shutdown(ipc_fd, SHUT_WR); + close(ipc_fd); } } - std::array ipc_fd; - pid_t worker_process_pid; + void shutdown_signal_watchers() { + ev_signal_stop(loop, &reopen_log_signalev); + ev_signal_stop(loop, &exec_binary_signalev); + ev_signal_stop(loop, &graceful_shutdown_signalev); + ev_signal_stop(loop, &reload_signalev); + } + + ev_signal reopen_log_signalev; + ev_signal exec_binary_signalev; + ev_signal graceful_shutdown_signalev; + ev_signal reload_signalev; + ev_child worker_process_childev; + struct ev_loop *loop; + pid_t worker_pid; + int ipc_fd; }; +namespace { +void reload_config(SignalServer *ssv); +} // namespace + +namespace { +std::deque> signal_servers; +} // namespace + +namespace { +void signal_server_add(std::unique_ptr ssv) { + signal_servers.push_back(std::move(ssv)); +} +} // namespace + +namespace { +void signal_server_remove(const SignalServer *ssv) { + for (auto it = std::begin(signal_servers); it != std::end(signal_servers); + ++it) { + auto &s = *it; + + if (s.get() != ssv) { + continue; + } + + signal_servers.erase(it); + break; + } +} +} // namespace + +namespace { +void signal_server_remove_all() { + std::deque>().swap(signal_servers); +} +} // namespace + +namespace { +// Send signal |signum| to all worker processes, and clears +// signal_servers. +void signal_server_kill(int signum) { + for (auto &s : signal_servers) { + if (s->worker_pid == -1) { + continue; + } + kill(s->worker_pid, signum); + } + signal_servers.clear(); +} +} // namespace + namespace { int chown_to_running_user(const char *path) { return chown(path, get_config()->uid, get_config()->gid); @@ -346,8 +462,7 @@ void exec_binary(SignalServer *ssv) { namespace { void ipc_send(SignalServer *ssv, uint8_t ipc_event) { ssize_t nwrite; - while ((nwrite = write(ssv->ipc_fd[1], &ipc_event, 1)) == -1 && - errno == EINTR) + while ((nwrite = write(ssv->ipc_fd, &ipc_event, 1)) == -1 && errno == EINTR) ; if (nwrite < 0) { @@ -377,7 +492,7 @@ void reopen_log(SignalServer *ssv) { namespace { void signal_cb(struct ev_loop *loop, ev_signal *w, int revents) { auto ssv = static_cast(w->data); - if (ssv->worker_process_pid == -1) { + if (ssv->worker_pid == -1) { ev_break(loop); return; } @@ -392,8 +507,11 @@ void signal_cb(struct ev_loop *loop, ev_signal *w, int revents) { case GRACEFUL_SHUTDOWN_SIGNAL: ipc_send(ssv, SHRPX_IPC_GRACEFUL_SHUTDOWN); return; + case RELOAD_SIGNAL: + reload_config(ssv); + return; default: - kill(ssv->worker_process_pid, w->signum); + signal_server_kill(w->signum); ev_break(loop); return; } @@ -402,24 +520,20 @@ void signal_cb(struct ev_loop *loop, ev_signal *w, int revents) { namespace { void worker_process_child_cb(struct ev_loop *loop, ev_child *w, int revents) { + auto ssv = static_cast(w->data); + log_chld(w->rpid, w->rstatus, "Worker process"); - ev_child_stop(loop, w); + auto pid = ssv->worker_pid; - ev_break(loop); + signal_server_remove(ssv); + + if (get_config()->last_worker_pid == pid) { + ev_break(loop); + } } } // namespace -struct InheritedAddr { - // IP address if TCP socket. Otherwise, UNIX domain socket path. - ImmutableString host; - uint16_t port; - // true if UNIX domain socket path - bool host_unix; - int fd; - bool used; -}; - namespace { int create_unix_domain_server_socket(UpstreamAddr &faddr, std::vector &iaddrs) { @@ -660,6 +774,70 @@ int create_tcp_server_socket(UpstreamAddr &faddr, } // namespace namespace { +// Returns array of InheritedAddr constructed from |config|. This +// function is intended to be used when reloading configuration, and +// |config| is usually a current configuration. +std::vector +get_inherited_addr_from_config(const Config *config) { + int rv; + std::vector iaddrs; + + auto &listenerconf = config->conn.listener; + + for (auto &addr : listenerconf.addrs) { + iaddrs.emplace_back(); + auto &iaddr = iaddrs.back(); + + if (addr.host_unix) { + iaddr.host = addr.host; + iaddr.host_unix = true; + iaddr.fd = addr.fd; + + continue; + } + + iaddr.port = addr.port; + iaddr.fd = addr.fd; + + // We have to getsockname/getnameinfo for fd, since we may have + // '*' appear in addr.host, which makes comparison against "real" + // address fail. + + sockaddr_union su; + socklen_t salen = sizeof(su); + + // We already added entry to iaddrs. Even if we got errors, we + // don't remove it. This is required because we have to close the + // socket if it is not reused. The empty host name usually does + // not match anything. + + if (getsockname(addr.fd, &su.sa, &salen) != 0) { + auto error = errno; + LOG(WARN) << "getsockname() syscall failed (fd=" << addr.fd + << "): " << strerror(error); + continue; + } + + std::array host; + rv = getnameinfo(&su.sa, salen, host.data(), host.size(), nullptr, 0, + NI_NUMERICHOST); + if (rv != 0) { + LOG(WARN) << "getnameinfo() failed (fd=" << addr.fd + << "): " << gai_strerror(rv); + continue; + } + + iaddr.host = host.data(); + } + + return iaddrs; +} +} // namespace + +namespace { +// Returns array of InheritedAddr constructed from environment +// variables. This function handles the old environment variable +// names used in 1.7.0 or earlier. std::vector get_inherited_addr_from_env() { int rv; std::vector iaddrs; @@ -808,7 +986,8 @@ std::vector get_inherited_addr_from_env() { } // namespace namespace { -void closeUnusedInheritedAddr(const std::vector &iaddrs) { +// Closes all sockets which are not reused. +void close_unused_inherited_addr(const std::vector &iaddrs) { for (auto &ia : iaddrs) { if (ia.used) { continue; @@ -820,8 +999,8 @@ void closeUnusedInheritedAddr(const std::vector &iaddrs) { } // namespace namespace { -int create_acceptor_socket(std::vector &iaddrs) { - auto &listenerconf = mod_config()->conn.listener; +int create_acceptor_socket(Config *config, std::vector &iaddrs) { + auto &listenerconf = config->conn.listener; for (auto &addr : listenerconf.addrs) { if (addr.host_unix) { @@ -829,7 +1008,7 @@ int create_acceptor_socket(std::vector &iaddrs) { return -1; } - if (get_config()->uid != 0) { + if (config->uid != 0) { // fd is not associated to inode, so we cannot use fchown(2) // here. https://lkml.org/lkml/2004/11/1/84 if (chown_to_running_user(addr.host.c_str()) == -1) { @@ -861,15 +1040,54 @@ int call_daemon() { } // namespace namespace { -pid_t fork_worker_process(SignalServer *ssv) { +// Opens IPC socket used to communicate with worker proess. The +// communication is unidirectional; that is main process sends +// messages to the worker process. On success, ipc_fd[0] is for +// reading, and ipc_fd[1] for writing, just like pipe(2). +int create_ipc_socket(std::array &ipc_fd) { + int rv; + + rv = pipe(ipc_fd.data()); + if (rv == -1) { + auto error = errno; + LOG(WARN) << "Failed to create pipe to communicate worker process: " + << strerror(error); + return -1; + } + + for (int i = 0; i < 2; ++i) { + auto fd = ipc_fd[i]; + util::make_socket_nonblocking(fd); + util::make_socket_closeonexec(fd); + } + + return 0; +} +} // namespace + +namespace { +// Creates worker process, and returns PID of worker process. On +// success, file descriptor for IPC (send only) is assigned to +// |main_ipc_fd|. +pid_t fork_worker_process(int &main_ipc_fd) { int rv; sigset_t oldset; + std::array ipc_fd; + + rv = create_ipc_socket(ipc_fd); + if (rv != 0) { + return -1; + } + rv = shrpx_signal_block_all(&oldset); if (rv != 0) { auto error = errno; LOG(ERROR) << "Blocking all signals failed: " << strerror(error); + close(ipc_fd[0]); + close(ipc_fd[1]); + return -1; } @@ -878,6 +1096,10 @@ pid_t fork_worker_process(SignalServer *ssv) { if (pid == 0) { ev_loop_fork(EV_DEFAULT); + // Remove all SignalServers to stop any registered watcher on + // default loop. + signal_server_remove_all(); + shrpx_signal_set_worker_proc_ign_handler(); rv = shrpx_signal_unblock_all(); @@ -888,8 +1110,8 @@ pid_t fork_worker_process(SignalServer *ssv) { _Exit(EXIT_FAILURE); } - close(ssv->ipc_fd[1]); - WorkerProcessConfig wpconf{ssv->ipc_fd[0]}; + close(ipc_fd[1]); + WorkerProcessConfig wpconf{ipc_fd[0]}; rv = worker_process_event_loop(&wpconf); if (rv != 0) { LOG(FATAL) << "Worker process returned error"; @@ -918,10 +1140,15 @@ pid_t fork_worker_process(SignalServer *ssv) { } if (pid == -1) { + close(ipc_fd[0]); + close(ipc_fd[1]); + return -1; } - close(ssv->ipc_fd[0]); + close(ipc_fd[0]); + + main_ipc_fd = ipc_fd[1]; LOG(NOTICE) << "Worker process [" << pid << "] spawned"; @@ -931,8 +1158,6 @@ pid_t fork_worker_process(SignalServer *ssv) { namespace { int event_loop() { - int rv; - shrpx_signal_set_master_proc_ign_handler(); if (get_config()->daemon) { @@ -950,55 +1175,27 @@ int event_loop() { redirect_stderr_to_errorlog(); } - SignalServer ssv; - - rv = pipe(ssv.ipc_fd.data()); - if (rv == -1) { - auto error = errno; - LOG(WARN) << "Failed to create pipe to communicate worker process: " - << strerror(error); - return -1; - } - - for (int i = 0; i < 2; ++i) { - auto fd = ssv.ipc_fd[i]; - util::make_socket_nonblocking(fd); - util::make_socket_closeonexec(fd); - } - auto iaddrs = get_inherited_addr_from_env(); - if (create_acceptor_socket(iaddrs) != 0) { + if (create_acceptor_socket(mod_config(), iaddrs) != 0) { return -1; } - closeUnusedInheritedAddr(iaddrs); + close_unused_inherited_addr(iaddrs); auto loop = ev_default_loop(get_config()->ev_loop_flags); - auto pid = fork_worker_process(&ssv); + int ipc_fd; + + auto pid = fork_worker_process(ipc_fd); if (pid == -1) { return -1; } - ssv.worker_process_pid = pid; + signal_server_add(make_unique(loop, pid, ipc_fd)); - constexpr auto signals = std::array{ - {REOPEN_LOG_SIGNAL, EXEC_BINARY_SIGNAL, GRACEFUL_SHUTDOWN_SIGNAL}}; - auto sigevs = std::array(); - - for (size_t i = 0; i < signals.size(); ++i) { - auto sigev = &sigevs[i]; - ev_signal_init(sigev, signal_cb, signals[i]); - sigev->data = &ssv; - ev_signal_start(loop, sigev); - } - - ev_child worker_process_childev; - ev_child_init(&worker_process_childev, worker_process_child_cb, pid, 0); - worker_process_childev.data = nullptr; - ev_child_start(loop, &worker_process_childev); + mod_config()->last_worker_pid = pid; // Write PID file when we are ready to accept connection from peer. // This makes easier to write restart script for nghttpx. Because @@ -1043,18 +1240,19 @@ constexpr auto DEFAULT_ACCESSLOG_FORMAT = StringRef::from_lit( } // namespace namespace { -void fill_default_config() { - *mod_config() = {}; +void fill_default_config(Config *config) { + *config = {}; - mod_config()->num_worker = 1; - mod_config()->conf_path = "/etc/nghttpx/nghttpx.conf"; - mod_config()->pid = getpid(); + config->num_worker = 1; + config->conf_path = "/etc/nghttpx/nghttpx.conf"; + config->pid = getpid(); + config->last_worker_pid = -1; if (ev_supported_backends() & ~ev_recommended_backends() & EVBACKEND_KQUEUE) { - mod_config()->ev_loop_flags = ev_recommended_backends() | EVBACKEND_KQUEUE; + config->ev_loop_flags = ev_recommended_backends() | EVBACKEND_KQUEUE; } - auto &tlsconf = mod_config()->tls; + auto &tlsconf = config->tls; { auto &ticketconf = tlsconf.ticket; { @@ -1089,7 +1287,7 @@ void fill_default_config() { tlsconf.session_timeout = std::chrono::hours(12); - auto &httpconf = mod_config()->http; + auto &httpconf = config->http; httpconf.server_name = StringRef::from_lit("nghttpx nghttp2/" NGHTTP2_VERSION); httpconf.no_host_rewrite = true; @@ -1098,7 +1296,7 @@ void fill_default_config() { httpconf.response_header_field_buffer = 64_k; httpconf.max_response_header_fields = 500; - auto &http2conf = mod_config()->http2; + auto &http2conf = config->http2; { auto &upstreamconf = http2conf.upstream; @@ -1143,7 +1341,7 @@ void fill_default_config() { nghttp2_option_set_peer_max_concurrent_streams(downstreamconf.option, 100); } - auto &loggingconf = mod_config()->logging; + auto &loggingconf = config->logging; { auto &accessconf = loggingconf.access; accessconf.format = parse_log_format(DEFAULT_ACCESSLOG_FORMAT); @@ -1154,7 +1352,7 @@ void fill_default_config() { loggingconf.syslog_facility = LOG_DAEMON; - auto &connconf = mod_config()->conn; + auto &connconf = config->conn; { auto &listenerconf = connconf.listener; { @@ -1198,7 +1396,7 @@ void fill_default_config() { downstreamconf.family = AF_UNSPEC; } - auto &apiconf = mod_config()->api; + auto &apiconf = config->api; apiconf.max_request_body = 16_k; } @@ -2038,23 +2236,17 @@ Misc: } // namespace namespace { -void process_options(int argc, char **argv, - std::vector> &cmdcfgs) { - if (conf_exists(get_config()->conf_path.c_str())) { +int process_options(Config *config, + std::vector> &cmdcfgs) { + if (conf_exists(config->conf_path.c_str())) { std::set include_set; - if (load_config(get_config()->conf_path.c_str(), include_set) == -1) { - LOG(FATAL) << "Failed to load configuration from " - << get_config()->conf_path; - exit(EXIT_FAILURE); + if (load_config(config, config->conf_path.c_str(), include_set) == -1) { + LOG(FATAL) << "Failed to load configuration from " << config->conf_path; + return -1; } assert(include_set.empty()); } - if (argc - optind >= 2) { - cmdcfgs.emplace_back(SHRPX_OPT_PRIVATE_KEY_FILE, StringRef{argv[optind++]}); - cmdcfgs.emplace_back(SHRPX_OPT_CERTIFICATE_FILE, StringRef{argv[optind++]}); - } - // Reopen log files using configurations in file reopen_log_files(); @@ -2062,16 +2254,16 @@ void process_options(int argc, char **argv, std::set include_set; for (auto &p : cmdcfgs) { - if (parse_config(mod_config(), p.first, p.second, include_set) == -1) { + if (parse_config(config, p.first, p.second, include_set) == -1) { LOG(FATAL) << "Failed to parse command-line argument."; - exit(EXIT_FAILURE); + return -1; } } assert(include_set.empty()); } - auto &loggingconf = get_config()->logging; + auto &loggingconf = config->logging; if (loggingconf.access.syslog || loggingconf.error.syslog) { openlog("nghttpx", LOG_NDELAY | LOG_NOWAIT | LOG_PID, @@ -2080,29 +2272,27 @@ void process_options(int argc, char **argv, if (reopen_log_files() != 0) { LOG(FATAL) << "Failed to open log file"; - exit(EXIT_FAILURE); + return -1; } redirect_stderr_to_errorlog(); - if (get_config()->uid != 0) { + if (config->uid != 0) { if (log_config()->accesslog_fd != -1 && - fchown(log_config()->accesslog_fd, get_config()->uid, - get_config()->gid) == -1) { + fchown(log_config()->accesslog_fd, config->uid, config->gid) == -1) { auto error = errno; LOG(WARN) << "Changing owner of access log file failed: " << strerror(error); } if (log_config()->errorlog_fd != -1 && - fchown(log_config()->errorlog_fd, get_config()->uid, - get_config()->gid) == -1) { + fchown(log_config()->errorlog_fd, config->uid, config->gid) == -1) { auto error = errno; LOG(WARN) << "Changing owner of error log file failed: " << strerror(error); } } - auto &http2conf = mod_config()->http2; + auto &http2conf = config->http2; { auto &dumpconf = http2conf.upstream.debug.dump; @@ -2113,12 +2303,12 @@ void process_options(int argc, char **argv, if (f == nullptr) { LOG(FATAL) << "Failed to open http2 upstream request header file: " << path; - exit(EXIT_FAILURE); + return -1; } dumpconf.request_header = f; - if (get_config()->uid != 0) { + if (config->uid != 0) { if (chown_to_running_user(path) == -1) { auto error = errno; LOG(WARN) << "Changing owner of http2 upstream request header file " @@ -2134,12 +2324,12 @@ void process_options(int argc, char **argv, if (f == nullptr) { LOG(FATAL) << "Failed to open http2 upstream response header file: " << path; - exit(EXIT_FAILURE); + return -1; } dumpconf.response_header = f; - if (get_config()->uid != 0) { + if (config->uid != 0) { if (chown_to_running_user(path) == -1) { auto error = errno; LOG(WARN) << "Changing owner of http2 upstream response header file" @@ -2149,7 +2339,7 @@ void process_options(int argc, char **argv, } } - auto &tlsconf = mod_config()->tls; + auto &tlsconf = config->tls; if (tlsconf.npn_list.empty()) { tlsconf.npn_list = util::parse_config_str_list(DEFAULT_NPN_LIST); @@ -2165,8 +2355,8 @@ void process_options(int argc, char **argv, tlsconf.bio_method = create_bio_method(); - auto &listenerconf = mod_config()->conn.listener; - auto &upstreamconf = mod_config()->conn.upstream; + auto &listenerconf = config->conn.listener; + auto &upstreamconf = config->conn.upstream; if (listenerconf.addrs.empty()) { UpstreamAddr addr{}; @@ -2187,7 +2377,7 @@ void process_options(int argc, char **argv, (tlsconf.private_key_file.empty() || tlsconf.cert_file.empty())) { print_usage(std::cerr); LOG(FATAL) << "Too few arguments"; - exit(EXIT_FAILURE); + return -1; } if (ssl::upstream_tls_enabled() && !tlsconf.ocsp.disabled) { @@ -2200,18 +2390,18 @@ void process_options(int argc, char **argv, } } - if (configure_downstream_group(mod_config(), get_config()->http2_proxy, false, - tlsconf) != 0) { - exit(EXIT_FAILURE); + if (configure_downstream_group(config, config->http2_proxy, false, tlsconf) != + 0) { + return -1; } - auto &proxy = mod_config()->downstream_http_proxy; + auto &proxy = config->downstream_http_proxy; if (!proxy.host.empty()) { auto hostport = util::make_hostport(StringRef{proxy.host}, proxy.port); if (resolve_hostname(&proxy.addr, proxy.host.c_str(), proxy.port, AF_UNSPEC) == -1) { LOG(FATAL) << "Resolving backend HTTP proxy address failed: " << hostport; - exit(EXIT_FAILURE); + return -1; } LOG(NOTICE) << "Backend HTTP proxy address: " << hostport << " -> " << util::to_numeric_addr(&proxy.addr); @@ -2227,7 +2417,7 @@ void process_options(int argc, char **argv, LOG(FATAL) << "Resolving memcached address for TLS session cache failed: " << hostport; - exit(EXIT_FAILURE); + return -1; } LOG(NOTICE) << "Memcached address for TLS session cache: " << hostport << " -> " << util::to_numeric_addr(&memcachedconf.addr); @@ -2247,7 +2437,7 @@ void process_options(int argc, char **argv, memcachedconf.port, memcachedconf.family) == -1) { LOG(FATAL) << "Resolving memcached address for TLS ticket key failed: " << hostport; - exit(EXIT_FAILURE); + return -1; } LOG(NOTICE) << "Memcached address for TLS ticket key: " << hostport << " -> " << util::to_numeric_addr(&memcachedconf.addr); @@ -2258,27 +2448,26 @@ void process_options(int argc, char **argv, } } - if (get_config()->rlimit_nofile) { - struct rlimit lim = {static_cast(get_config()->rlimit_nofile), - static_cast(get_config()->rlimit_nofile)}; + if (config->rlimit_nofile) { + struct rlimit lim = {static_cast(config->rlimit_nofile), + static_cast(config->rlimit_nofile)}; if (setrlimit(RLIMIT_NOFILE, &lim) != 0) { auto error = errno; LOG(WARN) << "Setting rlimit-nofile failed: " << strerror(error); } } - auto &fwdconf = mod_config()->http.forwarded; + auto &fwdconf = config->http.forwarded; if (fwdconf.by_node_type == FORWARDED_NODE_OBFUSCATED && fwdconf.by_obfuscated.empty()) { - std::random_device rd; std::mt19937 gen(rd()); auto &dst = fwdconf.by_obfuscated; dst = "_"; dst += util::random_alpha_digit(gen, SHRPX_OBFUSCATED_NODE_LENGTH); } - if (get_config()->http2.upstream.debug.frame_debug) { + if (config->http2.upstream.debug.frame_debug) { // To make it sync to logging set_output(stderr); if (isatty(fileno(stdout))) { @@ -2287,13 +2476,88 @@ void process_options(int argc, char **argv, reset_timer(); } - mod_config()->http2.upstream.callbacks = create_http2_upstream_callbacks(); - mod_config()->http2.downstream.callbacks = - create_http2_downstream_callbacks(); + config->http2.upstream.callbacks = create_http2_upstream_callbacks(); + config->http2.downstream.callbacks = create_http2_downstream_callbacks(); + + return 0; +} +} // namespace + +namespace { +void reload_config(SignalServer *ssv) { + int rv; + + LOG(NOTICE) << "Reloading configuration"; + + auto cur_config = get_config(); + auto new_config = make_unique(); + + fill_default_config(new_config.get()); + + new_config->conf_path = cur_config->conf_path; + new_config->argc = cur_config->argc; + new_config->argv = cur_config->argv; + new_config->original_argv = cur_config->original_argv; + new_config->cwd = cur_config->cwd; + + rv = process_options(new_config.get(), cmdcfgs); + if (rv != 0) { + LOG(ERROR) << "Failed to process new configuration"; + return; + } + + // daemon option is ignored here. + + auto iaddrs = get_inherited_addr_from_config(cur_config); + + if (create_acceptor_socket(new_config.get(), iaddrs) != 0) { + return; + } + + // TODO may leak socket if we get error later in this function + + // TODO loop is reused, and new_config->ev_loop_flags gets ignored + + auto loop = ssv->loop; + + int ipc_fd; + + auto pid = fork_worker_process(ipc_fd); + + if (pid == -1) { + LOG(ERROR) << "Failed to process new configuration"; + return; + } + + close_unused_inherited_addr(iaddrs); + + // Send last worker process a graceful shutdown notice + auto &last_ssv = signal_servers.back(); + ipc_send(last_ssv.get(), SHRPX_IPC_GRACEFUL_SHUTDOWN); + // We no longer use signals for this worker. + last_ssv->shutdown_signal_watchers(); + + signal_server_add(make_unique(loop, pid, ipc_fd)); + + new_config->last_worker_pid = pid; + + auto old_config = replace_config(new_config.release()); + + assert(cur_config == old_config); + + delete_config(old_config); + + // Now we use new configuration + + if (!get_config()->pid_file.empty()) { + save_pid(); + } } } // namespace int main(int argc, char **argv) { + int rv; + nghttp2::ssl::libssl_init(); #ifndef NOTHREADS @@ -2302,7 +2566,7 @@ int main(int argc, char **argv) { Log::set_severity_level(NOTICE); create_config(); - fill_default_config(); + fill_default_config(mod_config()); // make copy of stderr util::store_original_fds(); @@ -2333,7 +2597,6 @@ int main(int argc, char **argv) { exit(EXIT_FAILURE); } - std::vector> cmdcfgs; while (1) { static int flag = 0; static option long_options[] = { @@ -3121,7 +3384,15 @@ int main(int argc, char **argv) { } } - process_options(argc, argv, cmdcfgs); + if (argc - optind >= 2) { + cmdcfgs.emplace_back(SHRPX_OPT_PRIVATE_KEY_FILE, StringRef{argv[optind++]}); + cmdcfgs.emplace_back(SHRPX_OPT_CERTIFICATE_FILE, StringRef{argv[optind++]}); + } + + rv = process_options(mod_config(), cmdcfgs); + if (rv != 0) { + return -1; + } if (event_loop() != 0) { return -1; diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index f356a3fb..8caa35bd 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -69,8 +69,44 @@ const Config *get_config() { return config; } Config *mod_config() { return config; } +Config *replace_config(Config *new_config) { + std::swap(config, new_config); + return new_config; +} + void create_config() { config = new Config(); } +void delete_config(Config *config) { + if (config == nullptr) { + return; + } + + auto &http2conf = config->http2; + + auto &upstreamconf = http2conf.upstream; + + nghttp2_option_del(upstreamconf.option); + nghttp2_option_del(upstreamconf.alt_mode_option); + nghttp2_session_callbacks_del(upstreamconf.callbacks); + + auto &downstreamconf = http2conf.downstream; + + nghttp2_option_del(downstreamconf.option); + nghttp2_session_callbacks_del(downstreamconf.callbacks); + + auto &dumpconf = http2conf.upstream.debug.dump; + + if (dumpconf.request_header) { + fclose(dumpconf.request_header); + } + + if (dumpconf.response_header) { + fclose(dumpconf.response_header); + } + + delete config; +} + TicketKeys::~TicketKeys() { /* Erase keys from memory */ for (auto &key : keys) { @@ -2398,7 +2434,7 @@ int parse_config(Config *config, int optid, const StringRef &opt, } included_set.insert(optarg); - auto rv = load_config(optarg.c_str(), included_set); + auto rv = load_config(config, optarg.c_str(), included_set); included_set.erase(optarg); if (rv != 0) { @@ -2648,7 +2684,8 @@ int parse_config(Config *config, int optid, const StringRef &opt, return -1; } -int load_config(const char *filename, std::set &include_set) { +int load_config(Config *config, const char *filename, + std::set &include_set) { std::ifstream in(filename, std::ios::binary); if (!in) { LOG(ERROR) << "Could not open config file " << filename; @@ -2669,7 +2706,7 @@ int load_config(const char *filename, std::set &include_set) { } *eq = '\0'; - if (parse_config(mod_config(), StringRef{std::begin(line), eq}, + if (parse_config(config, StringRef{std::begin(line), eq}, StringRef{eq + 1, std::end(line)}, include_set) != 0) { return -1; } diff --git a/src/shrpx_config.h b/src/shrpx_config.h index 2ea76015..c9d9815d 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -727,6 +727,9 @@ struct Config { uid_t uid; gid_t gid; pid_t pid; + // With reloading feature, we may have multiple worker PIDs at the + // given moment. This field tracks the last worker PID. + pid_t last_worker_pid; bool verbose; bool daemon; bool http2_proxy; @@ -736,7 +739,11 @@ struct Config { const Config *get_config(); Config *mod_config(); +// Replaces the current config with given |new_config|. The old config is +// returned. +Config *replace_config(Config *new_config); void create_config(); +void delete_config(Config *config); // generated by gennghttpxfun.py enum { @@ -890,10 +897,11 @@ int parse_config(Config *config, const StringRef &opt, const StringRef &optarg, int parse_config(Config *config, int optid, const StringRef &opt, const StringRef &optarg, std::set &included_set); -// Loads configurations from |filename| and stores them in statically -// allocated Config object. This function returns 0 if it succeeds, or -// -1. See parse_config() for |include_set|. -int load_config(const char *filename, std::set &include_set); +// Loads configurations from |filename| and stores them in |config|. +// This function returns 0 if it succeeds, or -1. See parse_config() +// for |include_set|. +int load_config(Config *config, const char *filename, + std::set &include_set); // Parses header field in |optarg|. We expect header field is formed // like "NAME: VALUE". We require that NAME is non empty string. ":" diff --git a/src/shrpx_signal.cc b/src/shrpx_signal.cc index 33a6df6c..3030de38 100644 --- a/src/shrpx_signal.cc +++ b/src/shrpx_signal.cc @@ -114,8 +114,9 @@ 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}}; +constexpr auto worker_proc_ign_signals = + std::array{{REOPEN_LOG_SIGNAL, EXEC_BINARY_SIGNAL, + GRACEFUL_SHUTDOWN_SIGNAL, RELOAD_SIGNAL, SIGPIPE}}; } // namespace void shrpx_signal_set_master_proc_ign_handler() { diff --git a/src/shrpx_signal.h b/src/shrpx_signal.h index dcedd792..c1c61b3c 100644 --- a/src/shrpx_signal.h +++ b/src/shrpx_signal.h @@ -34,6 +34,7 @@ namespace shrpx { constexpr int REOPEN_LOG_SIGNAL = SIGUSR1; constexpr int EXEC_BINARY_SIGNAL = SIGUSR2; constexpr int GRACEFUL_SHUTDOWN_SIGNAL = SIGQUIT; +constexpr int RELOAD_SIGNAL = SIGHUP; // Blocks all signals. The previous signal mask is stored into // |oldset| if it is not nullptr. This function returns 0 if it From 494775a25df46d2a037e4c12dea565133a703463 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 31 Jul 2016 16:16:23 +0900 Subject: [PATCH 03/11] nghttpx: Rename SignalServer with WorkerProcess --- src/shrpx.cc | 82 ++++++++++++++++++++++++++-------------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index d6c9426d..ab427804 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -158,8 +158,8 @@ namespace { void worker_process_child_cb(struct ev_loop *loop, ev_child *w, int revents); } // namespace -struct SignalServer { - SignalServer(struct ev_loop *loop, pid_t worker_pid, int ipc_fd) +struct WorkerProcess { + WorkerProcess(struct ev_loop *loop, pid_t worker_pid, int ipc_fd) : loop(loop), worker_pid(worker_pid), ipc_fd(ipc_fd) { ev_signal_init(&reopen_log_signalev, signal_cb, REOPEN_LOG_SIGNAL); reopen_log_signalev.data = this; @@ -184,7 +184,7 @@ struct SignalServer { ev_child_start(loop, &worker_process_childev); } - ~SignalServer() { + ~WorkerProcess() { shutdown_signal_watchers(); ev_child_stop(loop, &worker_process_childev); @@ -213,52 +213,52 @@ struct SignalServer { }; namespace { -void reload_config(SignalServer *ssv); +void reload_config(WorkerProcess *wp); } // namespace namespace { -std::deque> signal_servers; +std::deque> worker_processes; } // namespace namespace { -void signal_server_add(std::unique_ptr ssv) { - signal_servers.push_back(std::move(ssv)); +void worker_process_add(std::unique_ptr wp) { + worker_processes.push_back(std::move(wp)); } } // namespace namespace { -void signal_server_remove(const SignalServer *ssv) { - for (auto it = std::begin(signal_servers); it != std::end(signal_servers); +void worker_process_remove(const WorkerProcess *wp) { + for (auto it = std::begin(worker_processes); it != std::end(worker_processes); ++it) { auto &s = *it; - if (s.get() != ssv) { + if (s.get() != wp) { continue; } - signal_servers.erase(it); + worker_processes.erase(it); break; } } } // namespace namespace { -void signal_server_remove_all() { - std::deque>().swap(signal_servers); +void worker_process_remove_all() { + std::deque>().swap(worker_processes); } } // namespace namespace { // Send signal |signum| to all worker processes, and clears -// signal_servers. -void signal_server_kill(int signum) { - for (auto &s : signal_servers) { +// worker_processes. +void worker_process_kill(int signum) { + for (auto &s : worker_processes) { if (s->worker_pid == -1) { continue; } kill(s->worker_pid, signum); } - signal_servers.clear(); + worker_processes.clear(); } } // namespace @@ -330,7 +330,7 @@ void save_pid() { } // namespace namespace { -void exec_binary(SignalServer *ssv) { +void exec_binary(WorkerProcess *wp) { int rv; sigset_t oldset; @@ -460,9 +460,9 @@ void exec_binary(SignalServer *ssv) { } // namespace namespace { -void ipc_send(SignalServer *ssv, uint8_t ipc_event) { +void ipc_send(WorkerProcess *wp, uint8_t ipc_event) { ssize_t nwrite; - while ((nwrite = write(ssv->ipc_fd, &ipc_event, 1)) == -1 && errno == EINTR) + while ((nwrite = write(wp->ipc_fd, &ipc_event, 1)) == -1 && errno == EINTR) ; if (nwrite < 0) { @@ -480,38 +480,38 @@ void ipc_send(SignalServer *ssv, uint8_t ipc_event) { } // namespace namespace { -void reopen_log(SignalServer *ssv) { +void reopen_log(WorkerProcess *wp) { LOG(NOTICE) << "Reopening log files: master process"; (void)reopen_log_files(); redirect_stderr_to_errorlog(); - ipc_send(ssv, SHRPX_IPC_REOPEN_LOG); + ipc_send(wp, SHRPX_IPC_REOPEN_LOG); } } // namespace namespace { void signal_cb(struct ev_loop *loop, ev_signal *w, int revents) { - auto ssv = static_cast(w->data); - if (ssv->worker_pid == -1) { + auto wp = static_cast(w->data); + if (wp->worker_pid == -1) { ev_break(loop); return; } switch (w->signum) { case REOPEN_LOG_SIGNAL: - reopen_log(ssv); + reopen_log(wp); return; case EXEC_BINARY_SIGNAL: - exec_binary(ssv); + exec_binary(wp); return; case GRACEFUL_SHUTDOWN_SIGNAL: - ipc_send(ssv, SHRPX_IPC_GRACEFUL_SHUTDOWN); + ipc_send(wp, SHRPX_IPC_GRACEFUL_SHUTDOWN); return; case RELOAD_SIGNAL: - reload_config(ssv); + reload_config(wp); return; default: - signal_server_kill(w->signum); + worker_process_kill(w->signum); ev_break(loop); return; } @@ -520,13 +520,13 @@ void signal_cb(struct ev_loop *loop, ev_signal *w, int revents) { namespace { void worker_process_child_cb(struct ev_loop *loop, ev_child *w, int revents) { - auto ssv = static_cast(w->data); + auto wp = static_cast(w->data); log_chld(w->rpid, w->rstatus, "Worker process"); - auto pid = ssv->worker_pid; + auto pid = wp->worker_pid; - signal_server_remove(ssv); + worker_process_remove(wp); if (get_config()->last_worker_pid == pid) { ev_break(loop); @@ -1096,9 +1096,9 @@ pid_t fork_worker_process(int &main_ipc_fd) { if (pid == 0) { ev_loop_fork(EV_DEFAULT); - // Remove all SignalServers to stop any registered watcher on + // Remove all WorkerProcesses to stop any registered watcher on // default loop. - signal_server_remove_all(); + worker_process_remove_all(); shrpx_signal_set_worker_proc_ign_handler(); @@ -1193,7 +1193,7 @@ int event_loop() { return -1; } - signal_server_add(make_unique(loop, pid, ipc_fd)); + worker_process_add(make_unique(loop, pid, ipc_fd)); mod_config()->last_worker_pid = pid; @@ -2484,7 +2484,7 @@ int process_options(Config *config, } // namespace namespace { -void reload_config(SignalServer *ssv) { +void reload_config(WorkerProcess *wp) { int rv; LOG(NOTICE) << "Reloading configuration"; @@ -2518,7 +2518,7 @@ void reload_config(SignalServer *ssv) { // TODO loop is reused, and new_config->ev_loop_flags gets ignored - auto loop = ssv->loop; + auto loop = wp->loop; int ipc_fd; @@ -2532,12 +2532,12 @@ void reload_config(SignalServer *ssv) { close_unused_inherited_addr(iaddrs); // Send last worker process a graceful shutdown notice - auto &last_ssv = signal_servers.back(); - ipc_send(last_ssv.get(), SHRPX_IPC_GRACEFUL_SHUTDOWN); + auto &last_wp = worker_processes.back(); + ipc_send(last_wp.get(), SHRPX_IPC_GRACEFUL_SHUTDOWN); // We no longer use signals for this worker. - last_ssv->shutdown_signal_watchers(); + last_wp->shutdown_signal_watchers(); - signal_server_add(make_unique(loop, pid, ipc_fd)); + worker_process_add(make_unique(loop, pid, ipc_fd)); new_config->last_worker_pid = pid; From b9b648e0ed9a2a68478bda366f6b3fde520b4c67 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 31 Jul 2016 16:20:00 +0900 Subject: [PATCH 04/11] nghttpx: Remove last_worker_pid from Config The last_worker_pid is known by inspecting the last entry of worker_processes. --- src/shrpx.cc | 19 +++++++++++++------ src/shrpx_config.h | 3 --- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index ab427804..77fb9814 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -262,6 +262,18 @@ void worker_process_kill(int signum) { } } // namespace +namespace { +// Returns the last PID of worker process. Returns -1 if there is no +// worker process at the moment. +int worker_process_last_pid() { + if (worker_processes.empty()) { + return -1; + } + + return worker_processes.back()->worker_pid; +} +} // namespace + namespace { int chown_to_running_user(const char *path) { return chown(path, get_config()->uid, get_config()->gid); @@ -528,7 +540,7 @@ void worker_process_child_cb(struct ev_loop *loop, ev_child *w, int revents) { worker_process_remove(wp); - if (get_config()->last_worker_pid == pid) { + if (worker_process_last_pid() == pid) { ev_break(loop); } } @@ -1195,8 +1207,6 @@ int event_loop() { worker_process_add(make_unique(loop, pid, ipc_fd)); - mod_config()->last_worker_pid = pid; - // Write PID file when we are ready to accept connection from peer. // This makes easier to write restart script for nghttpx. Because // when we know that PID file is recreated, it means we can send @@ -1246,7 +1256,6 @@ void fill_default_config(Config *config) { config->num_worker = 1; config->conf_path = "/etc/nghttpx/nghttpx.conf"; config->pid = getpid(); - config->last_worker_pid = -1; if (ev_supported_backends() & ~ev_recommended_backends() & EVBACKEND_KQUEUE) { config->ev_loop_flags = ev_recommended_backends() | EVBACKEND_KQUEUE; @@ -2539,8 +2548,6 @@ void reload_config(WorkerProcess *wp) { worker_process_add(make_unique(loop, pid, ipc_fd)); - new_config->last_worker_pid = pid; - auto old_config = replace_config(new_config.release()); assert(cur_config == old_config); diff --git a/src/shrpx_config.h b/src/shrpx_config.h index c9d9815d..d3c664ac 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -727,9 +727,6 @@ struct Config { uid_t uid; gid_t gid; pid_t pid; - // With reloading feature, we may have multiple worker PIDs at the - // given moment. This field tracks the last worker PID. - pid_t last_worker_pid; bool verbose; bool daemon; bool http2_proxy; From fb49182c29ac708275a3584b1309e6898e9e3b76 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 31 Jul 2016 16:34:55 +0900 Subject: [PATCH 05/11] nghttpx: Move original_argv, argv, argc, and cmdcfgs to StartupConfig --- src/shrpx.cc | 63 +++++++++++++++++++++++++++------------------- src/shrpx_config.h | 4 --- 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 77fb9814..0a4fcfe8 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -130,6 +130,25 @@ constexpr auto ENV_ACCEPT_PREFIX = StringRef::from_lit("NGHTTPX_ACCEPT_"); #endif #endif +// This configuration is fixed at the first startup of the main +// process, and does not change after subsequent reloadings. +struct StartupConfig { + // This contains all options given in command-line. + std::vector> cmdcfgs; + // The current working directory where this process started. + char *cwd; + // The pointer to original argv (not sure why we have this?) + char **original_argv; + // The pointer to argv, this is a deep copy of original argv. + char **argv; + // The number of elements in argv. + int argc; +}; + +namespace { +StartupConfig suconfig; +} // namespace + struct InheritedAddr { // IP address if TCP socket. Otherwise, UNIX domain socket path. ImmutableString host; @@ -144,12 +163,6 @@ namespace { std::random_device rd; } // namespace -namespace { -// This contains all options given in command-line. Make it static so -// that we can use it in reloading. -std::vector> cmdcfgs; -} // namespace - namespace { void signal_cb(struct ev_loop *loop, ev_signal *w, int revents); } // namespace @@ -388,21 +401,21 @@ void exec_binary(WorkerProcess *wp) { _Exit(EXIT_FAILURE); } - auto exec_path = util::get_exec_path(get_config()->argc, get_config()->argv, - get_config()->cwd); + auto exec_path = + util::get_exec_path(suconfig.argc, suconfig.argv, suconfig.cwd); if (!exec_path) { LOG(ERROR) << "Could not resolve the executable path"; _Exit(EXIT_FAILURE); } - auto argv = make_unique(get_config()->argc + 1); + auto argv = make_unique(suconfig.argc + 1); argv[0] = exec_path; - for (int i = 1; i < get_config()->argc; ++i) { - argv[i] = get_config()->argv[i]; + for (int i = 1; i < suconfig.argc; ++i) { + argv[i] = suconfig.argv[i]; } - argv[get_config()->argc] = nullptr; + argv[suconfig.argc] = nullptr; size_t envlen = 0; for (char **p = environ; *p; ++p, ++envlen) @@ -2504,19 +2517,15 @@ void reload_config(WorkerProcess *wp) { fill_default_config(new_config.get()); new_config->conf_path = cur_config->conf_path; - new_config->argc = cur_config->argc; - new_config->argv = cur_config->argv; - new_config->original_argv = cur_config->original_argv; - new_config->cwd = cur_config->cwd; + // daemon option is ignored here. + new_config->daemon = cur_config->daemon; - rv = process_options(new_config.get(), cmdcfgs); + rv = process_options(new_config.get(), suconfig.cmdcfgs); if (rv != 0) { LOG(ERROR) << "Failed to process new configuration"; return; } - // daemon option is ignored here. - auto iaddrs = get_inherited_addr_from_config(cur_config); if (create_acceptor_socket(new_config.get(), iaddrs) != 0) { @@ -2582,28 +2591,30 @@ int main(int argc, char **argv) { // log errors/warnings while reading configuration files. reopen_log_files(); - mod_config()->original_argv = argv; + suconfig.original_argv = argv; // We have to copy argv, since getopt_long may change its content. - mod_config()->argc = argc; - mod_config()->argv = new char *[argc]; + suconfig.argc = argc; + suconfig.argv = new char *[argc]; for (int i = 0; i < argc; ++i) { - mod_config()->argv[i] = strdup(argv[i]); - if (mod_config()->argv[i] == nullptr) { + suconfig.argv[i] = strdup(argv[i]); + if (suconfig.argv[i] == nullptr) { auto error = errno; LOG(FATAL) << "failed to copy argv: " << strerror(error); exit(EXIT_FAILURE); } } - mod_config()->cwd = getcwd(nullptr, 0); - if (mod_config()->cwd == nullptr) { + suconfig.cwd = getcwd(nullptr, 0); + if (suconfig.cwd == nullptr) { auto error = errno; LOG(FATAL) << "failed to get current working directory: errno=" << error; exit(EXIT_FAILURE); } + auto &cmdcfgs = suconfig.cmdcfgs; + while (1) { static int flag = 0; static option long_options[] = { diff --git a/src/shrpx_config.h b/src/shrpx_config.h index d3c664ac..e4a4c36b 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -717,13 +717,9 @@ struct Config { ImmutableString conf_path; ImmutableString user; ImmutableString mruby_file; - char **original_argv; - char **argv; - char *cwd; size_t num_worker; size_t padding; size_t rlimit_nofile; - int argc; uid_t uid; gid_t gid; pid_t pid; From 22570b7260fafc9f7da8f057336d219cc5adb495 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 31 Jul 2016 18:47:03 +0900 Subject: [PATCH 06/11] nghttpx: Close fd when error occurred in reload operation This commit also fixes the bug that old configuration is still used for worker process. The another bug fix is that inherited, but not used fd is not closed in worker process. That makes reloading next configuration fail if it contains the address which are leaked into worker process. --- src/shrpx.cc | 70 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 20 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 0a4fcfe8..d0a97b6b 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -805,13 +805,14 @@ namespace { std::vector get_inherited_addr_from_config(const Config *config) { int rv; - std::vector iaddrs; auto &listenerconf = config->conn.listener; + std::vector iaddrs(listenerconf.addrs.size()); + + size_t idx = 0; for (auto &addr : listenerconf.addrs) { - iaddrs.emplace_back(); - auto &iaddr = iaddrs.back(); + auto &iaddr = iaddrs[idx++]; if (addr.host_unix) { iaddr.host = addr.host; @@ -1093,8 +1094,11 @@ int create_ipc_socket(std::array &ipc_fd) { namespace { // Creates worker process, and returns PID of worker process. On // success, file descriptor for IPC (send only) is assigned to -// |main_ipc_fd|. -pid_t fork_worker_process(int &main_ipc_fd) { +// |main_ipc_fd|. In child process, we will close file descriptors +// which are inherited from previous configuration/process, but not +// used in the current configuration. +pid_t fork_worker_process(int &main_ipc_fd, + const std::vector &iaddrs) { int rv; sigset_t oldset; @@ -1125,6 +1129,8 @@ pid_t fork_worker_process(int &main_ipc_fd) { // default loop. worker_process_remove_all(); + close_unused_inherited_addr(iaddrs); + shrpx_signal_set_worker_proc_ign_handler(); rv = shrpx_signal_unblock_all(); @@ -1212,7 +1218,7 @@ int event_loop() { int ipc_fd; - auto pid = fork_worker_process(ipc_fd); + auto pid = fork_worker_process(ipc_fd, {}); if (pid == -1) { return -1; @@ -2505,6 +2511,27 @@ int process_options(Config *config, } } // namespace +namespace { +// Closes file descriptor which are opened for listeners in config, +// and which are not inherited from |iaddrs|. +void close_not_inherited_fd(Config *config, + const std::vector &iaddrs) { + auto &listenerconf = config->conn.listener; + + for (auto &addr : listenerconf.addrs) { + auto inherited = std::find_if( + std::begin(iaddrs), std::end(iaddrs), + [&addr](const InheritedAddr &iaddr) { return addr.fd == iaddr.fd; }); + + if (inherited != std::end(iaddrs)) { + continue; + } + + close(addr.fd); + } +} +} // namespace + namespace { void reload_config(WorkerProcess *wp) { int rv; @@ -2519,6 +2546,8 @@ void reload_config(WorkerProcess *wp) { new_config->conf_path = cur_config->conf_path; // daemon option is ignored here. new_config->daemon = cur_config->daemon; + // loop is reused, and ev_loop_flags gets ignored + new_config->ev_loop_flags = cur_config->ev_loop_flags; rv = process_options(new_config.get(), suconfig.cmdcfgs); if (rv != 0) { @@ -2529,24 +2558,33 @@ void reload_config(WorkerProcess *wp) { auto iaddrs = get_inherited_addr_from_config(cur_config); if (create_acceptor_socket(new_config.get(), iaddrs) != 0) { + close_not_inherited_fd(new_config.get(), iaddrs); return; } - // TODO may leak socket if we get error later in this function - - // TODO loop is reused, and new_config->ev_loop_flags gets ignored - - auto loop = wp->loop; + // According to libev documentation, flags are ignored since we have + // already created first default loop. + auto loop = ev_default_loop(new_config->ev_loop_flags); int ipc_fd; - auto pid = fork_worker_process(ipc_fd); + // fork_worker_process and forked child process assumes new + // configuration can be obtained from get_config(). + + auto old_config = replace_config(new_config.get()); + + auto pid = fork_worker_process(ipc_fd, iaddrs); if (pid == -1) { LOG(ERROR) << "Failed to process new configuration"; + close_not_inherited_fd(new_config.get(), iaddrs); + replace_config(old_config); return; } + new_config.release(); + delete_config(old_config); + close_unused_inherited_addr(iaddrs); // Send last worker process a graceful shutdown notice @@ -2557,14 +2595,6 @@ void reload_config(WorkerProcess *wp) { worker_process_add(make_unique(loop, pid, ipc_fd)); - auto old_config = replace_config(new_config.release()); - - assert(cur_config == old_config); - - delete_config(old_config); - - // Now we use new configuration - if (!get_config()->pid_file.empty()) { save_pid(); } From 8c3e864989ae6773e960206332f5d6326a7456d7 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 31 Jul 2016 19:01:29 +0900 Subject: [PATCH 07/11] nghttpx: Define ~Config for automatic clean up with std::unique_ptr Now config global is backed with std::unique_ptr. configuration swapping dance is now a bit cleaner, but YMMV. --- src/shrpx.cc | 9 ++++----- src/shrpx_config.cc | 30 +++++++++++------------------- src/shrpx_config.h | 5 +++-- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index d0a97b6b..6b330da2 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -2571,20 +2571,19 @@ void reload_config(WorkerProcess *wp) { // fork_worker_process and forked child process assumes new // configuration can be obtained from get_config(). - auto old_config = replace_config(new_config.get()); + auto old_config = replace_config(std::move(new_config)); auto pid = fork_worker_process(ipc_fd, iaddrs); if (pid == -1) { LOG(ERROR) << "Failed to process new configuration"; + + new_config = replace_config(std::move(old_config)); close_not_inherited_fd(new_config.get(), iaddrs); - replace_config(old_config); + return; } - new_config.release(); - delete_config(old_config); - close_unused_inherited_addr(iaddrs); // Send last worker process a graceful shutdown notice diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 8caa35bd..121ebbab 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -60,41 +60,35 @@ namespace shrpx { namespace { -Config *config = nullptr; +std::unique_ptr config; } // namespace constexpr auto SHRPX_UNIX_PATH_PREFIX = StringRef::from_lit("unix:"); -const Config *get_config() { return config; } +const Config *get_config() { return config.get(); } -Config *mod_config() { return config; } +Config *mod_config() { return config.get(); } -Config *replace_config(Config *new_config) { - std::swap(config, new_config); - return new_config; +std::unique_ptr replace_config(std::unique_ptr another) { + config.swap(another); + return another; } -void create_config() { config = new Config(); } +void create_config() { config = make_unique(); } -void delete_config(Config *config) { - if (config == nullptr) { - return; - } - - auto &http2conf = config->http2; - - auto &upstreamconf = http2conf.upstream; +Config::~Config() { + auto &upstreamconf = http2.upstream; nghttp2_option_del(upstreamconf.option); nghttp2_option_del(upstreamconf.alt_mode_option); nghttp2_session_callbacks_del(upstreamconf.callbacks); - auto &downstreamconf = http2conf.downstream; + auto &downstreamconf = http2.downstream; nghttp2_option_del(downstreamconf.option); nghttp2_session_callbacks_del(downstreamconf.callbacks); - auto &dumpconf = http2conf.upstream.debug.dump; + auto &dumpconf = http2.upstream.debug.dump; if (dumpconf.request_header) { fclose(dumpconf.request_header); @@ -103,8 +97,6 @@ void delete_config(Config *config) { if (dumpconf.response_header) { fclose(dumpconf.response_header); } - - delete config; } TicketKeys::~TicketKeys() { diff --git a/src/shrpx_config.h b/src/shrpx_config.h index e4a4c36b..d0645ff3 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -706,6 +706,8 @@ struct APIConfig { }; struct Config { + ~Config(); + HttpProxy downstream_http_proxy; HttpConfig http; Http2Config http2; @@ -734,9 +736,8 @@ const Config *get_config(); Config *mod_config(); // Replaces the current config with given |new_config|. The old config is // returned. -Config *replace_config(Config *new_config); +std::unique_ptr replace_config(std::unique_ptr new_config); void create_config(); -void delete_config(Config *config); // generated by gennghttpxfun.py enum { From 9a8e9815c9fae4fd30588750dd1f8ef23fb8aae5 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 31 Jul 2016 20:26:03 +0900 Subject: [PATCH 08/11] nghttpx: Cleanup --- src/shrpx.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 6b330da2..4bfe35ce 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -271,7 +271,7 @@ void worker_process_kill(int signum) { } kill(s->worker_pid, signum); } - worker_processes.clear(); + worker_process_remove_all(); } } // namespace @@ -355,7 +355,7 @@ void save_pid() { } // namespace namespace { -void exec_binary(WorkerProcess *wp) { +void exec_binary() { int rv; sigset_t oldset; @@ -527,7 +527,7 @@ void signal_cb(struct ev_loop *loop, ev_signal *w, int revents) { reopen_log(wp); return; case EXEC_BINARY_SIGNAL: - exec_binary(wp); + exec_binary(); return; case GRACEFUL_SHUTDOWN_SIGNAL: ipc_send(wp, SHRPX_IPC_GRACEFUL_SHUTDOWN); From e2906025c86aeaed9ef02c3c54a56f2bb209711f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 31 Jul 2016 20:35:10 +0900 Subject: [PATCH 09/11] nghttpx: Don't exit from save_pid and set_alpn_prefs --- src/shrpx.cc | 16 ++++++++++------ src/shrpx_ssl.cc | 14 ++++++-------- src/shrpx_ssl.h | 4 ++-- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 4bfe35ce..65cd6f84 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -294,7 +294,7 @@ int chown_to_running_user(const char *path) { } // namespace namespace { -void save_pid() { +int save_pid() { constexpr auto SUFFIX = StringRef::from_lit(".XXXXXX"); auto &pid_file = get_config()->pid_file; @@ -313,7 +313,7 @@ void save_pid() { auto error = errno; LOG(ERROR) << "Could not save PID to file " << pid_file << ": " << strerror(error); - exit(EXIT_FAILURE); + return -1; } auto content = util::utos(get_config()->pid) + '\n'; @@ -322,14 +322,14 @@ void save_pid() { auto error = errno; LOG(ERROR) << "Could not save PID to file " << pid_file << ": " << strerror(error); - exit(EXIT_FAILURE); + return -1; } if (fsync(fd) == -1) { auto error = errno; LOG(ERROR) << "Could not save PID to file " << pid_file << ": " << strerror(error); - exit(EXIT_FAILURE); + return -1; } close(fd); @@ -341,7 +341,7 @@ void save_pid() { unlink(temp_path); - exit(EXIT_FAILURE); + return -1; } if (get_config()->uid != 0) { @@ -351,6 +351,8 @@ void save_pid() { << " failed: " << strerror(error); } } + + return 0; } } // namespace @@ -2379,7 +2381,9 @@ int process_options(Config *config, tlsconf.tls_proto_mask = ssl::create_tls_proto_mask(tlsconf.tls_proto_list); - tlsconf.alpn_prefs = ssl::set_alpn_prefs(tlsconf.npn_list); + if (ssl::set_alpn_prefs(tlsconf.alpn_prefs, tlsconf.npn_list) != 0) { + return -1; + } tlsconf.bio_method = create_bio_method(); diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index fa09e137..469338a3 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -94,16 +94,14 @@ int verify_callback(int preverify_ok, X509_STORE_CTX *ctx) { } } // namespace -// This function is meant be called from master process, hence the -// call exit(3). -std::vector -set_alpn_prefs(const std::vector &protos) { +int set_alpn_prefs(std::vector &out, + const std::vector &protos) { size_t len = 0; for (const auto &proto : protos) { if (proto.size() > 255) { LOG(FATAL) << "Too long ALPN identifier: " << proto.size(); - exit(EXIT_FAILURE); + return -1; } len += 1 + proto.size(); @@ -111,10 +109,10 @@ set_alpn_prefs(const std::vector &protos) { if (len > (1 << 16) - 1) { LOG(FATAL) << "Too long ALPN identifier list: " << len; - exit(EXIT_FAILURE); + return -1; } - auto out = std::vector(len); + out.resize(len); auto ptr = out.data(); for (const auto &proto : protos) { @@ -123,7 +121,7 @@ set_alpn_prefs(const std::vector &protos) { ptr += proto.size(); } - return out; + return 0; } namespace { diff --git a/src/shrpx_ssl.h b/src/shrpx_ssl.h index 5f31c7a7..5138fff5 100644 --- a/src/shrpx_ssl.h +++ b/src/shrpx_ssl.h @@ -181,8 +181,8 @@ bool check_http2_requirement(SSL *ssl); // passed to SSL_CTX_set_options(). long int create_tls_proto_mask(const std::vector &tls_proto_list); -std::vector -set_alpn_prefs(const std::vector &protos); +int set_alpn_prefs(std::vector &out, + const std::vector &protos); // Setups server side SSL_CTX. This function inspects get_config() // and if upstream_no_tls is true, returns nullptr. Otherwise From 54f640f3e120dcf53c8194bc84898e14c4922a83 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 31 Jul 2016 20:50:07 +0900 Subject: [PATCH 10/11] nghttpx: Update doc --- src/shrpx.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 65cd6f84..98985c64 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -2517,7 +2517,7 @@ int process_options(Config *config, namespace { // Closes file descriptor which are opened for listeners in config, -// and which are not inherited from |iaddrs|. +// and are not inherited from |iaddrs|. void close_not_inherited_fd(Config *config, const std::vector &iaddrs) { auto &listenerconf = config->conn.listener; From d7c9015d8b72dd0803fbdfdd5b1f24130c2e1eef Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 31 Jul 2016 20:59:06 +0900 Subject: [PATCH 11/11] Update doc --- doc/nghttpx.h2r | 9 ++++++++- doc/sources/nghttpx-howto.rst | 3 +++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/doc/nghttpx.h2r b/doc/nghttpx.h2r index f8dc305b..e7fd9b52 100644 --- a/doc/nghttpx.h2r +++ b/doc/nghttpx.h2r @@ -49,6 +49,9 @@ SIGQUIT accepting connection. After all connections are handled, nghttpx exits. +SIGHUP + Reload configuration file given in :option:`--conf`. + SIGUSR1 Reopen log files. @@ -56,7 +59,11 @@ SIGUSR2 Fork and execute nghttpx. It will execute the binary in the same path with same command-line arguments and environment variables. After new process comes up, sending SIGQUIT to the original process - to perform hot swapping. + to perform hot swapping. The difference between SIGUSR2 + SIGQUIT + and SIGHUP is that former is usually used to execute new binary, and + the master process is newly spawned. On the other hand, the latter + just reloads configuration file, and the same master process + continues to exist. .. note:: diff --git a/doc/sources/nghttpx-howto.rst b/doc/sources/nghttpx-howto.rst index 28961ea8..15376b8d 100644 --- a/doc/sources/nghttpx-howto.rst +++ b/doc/sources/nghttpx-howto.rst @@ -236,6 +236,9 @@ all existing frontend connections are done, the current process will exit. At this point, only new nghttpx process exists and serves incoming requests. +If you want to just reload configuration file without executing new +binary, send SIGHUP to nghttpx master process. + Re-opening log files --------------------