nghttpx: Rename WorkerConfig as LogConfig

This is a sign that we only use thread-local storage for logging only.

src/Makefile.am

@@ -114,7 +114,7 @@ NGHTTPX_SRCS = \
 	shrpx_io_control.cc shrpx_io_control.h \
 	shrpx_ssl.cc shrpx_ssl.h \
 	shrpx_worker.cc shrpx_worker.h \
-	shrpx_worker_config.cc shrpx_worker_config.h \
+	shrpx_log_config.cc shrpx_log_config.h \
 	shrpx_connect_blocker.cc shrpx_connect_blocker.h \
 	shrpx_downstream_connection_pool.cc shrpx_downstream_connection_pool.h \
 	shrpx_rate_limit.cc shrpx_rate_limit.h \

src/shrpx.cc

@@ -62,7 +62,7 @@
 #include "shrpx_config.h"
 #include "shrpx_connection_handler.h"
 #include "shrpx_ssl.h"
-#include "shrpx_worker_config.h"
+#include "shrpx_log_config.h"
 #include "shrpx_worker.h"
 #include "shrpx_accept_handler.h"
 #include "shrpx_http2_upstream.h"
@@ -418,7 +418,7 @@ void reopen_log_signal_cb(struct ev_loop *loop, ev_signal *w, int revents) {
   auto conn_handler = static_cast<ConnectionHandler *>(w->data);
 
   if (LOG_ENABLED(INFO)) {
-    LOG(INFO) << "Reopening log files: worker_info(" << worker_config << ")";
+    LOG(INFO) << "Reopening log files: main";
   }
 
   (void)reopen_log_files();
@@ -1884,16 +1884,16 @@ int main(int argc, char **argv) {
   }
 
   if (get_config()->uid != 0) {
-    if (worker_config->accesslog_fd != -1 &&
-        fchown(worker_config->accesslog_fd, get_config()->uid,
+    if (log_config->accesslog_fd != -1 &&
+        fchown(log_config->accesslog_fd, get_config()->uid,
                get_config()->gid) == -1) {
       auto error = errno;
       LOG(WARN) << "Changing owner of access log file failed: "
                 << strerror(error);
     }
-    if (worker_config->errorlog_fd != -1 &&
-        fchown(worker_config->errorlog_fd, get_config()->uid,
-               get_config()->gid) == -1) {
+    if (log_config->errorlog_fd != -1 &&
+        fchown(log_config->errorlog_fd, get_config()->uid, get_config()->gid) ==
+            -1) {
       auto error = errno;
       LOG(WARN) << "Changing owner of error log file failed: "
                 << strerror(error);

src/shrpx_client_handler.cc

@@ -35,7 +35,6 @@
 #include "shrpx_http2_downstream_connection.h"
 #include "shrpx_ssl.h"
 #include "shrpx_worker.h"
-#include "shrpx_worker_config.h"
 #include "shrpx_downstream_connection_pool.h"
 #include "shrpx_downstream.h"
 #ifdef HAVE_SPDYLAY

src/shrpx_connection_handler.cc

@@ -32,7 +32,6 @@
 #include "shrpx_client_handler.h"
 #include "shrpx_ssl.h"
 #include "shrpx_worker.h"
-#include "shrpx_worker_config.h"
 #include "shrpx_config.h"
 #include "shrpx_http2_session.h"
 #include "shrpx_connect_blocker.h"

src/shrpx_http.cc

@@ -26,7 +26,6 @@
 
 #include "shrpx_config.h"
 #include "shrpx_log.h"
-#include "shrpx_worker_config.h"
 #include "http2.h"
 #include "util.h"
 

src/shrpx_http2_downstream_connection.cc

@@ -35,7 +35,6 @@
 #include "shrpx_error.h"
 #include "shrpx_http.h"
 #include "shrpx_http2_session.h"
-#include "shrpx_worker_config.h"
 #include "http2.h"
 #include "util.h"
 

src/shrpx_http2_session.cc

@@ -39,7 +39,6 @@
 #include "shrpx_client_handler.h"
 #include "shrpx_ssl.h"
 #include "shrpx_http.h"
-#include "shrpx_worker_config.h"
 #include "http2.h"
 #include "util.h"
 #include "base64.h"

src/shrpx_http2_upstream.cc

@@ -35,7 +35,6 @@
 #include "shrpx_downstream_connection.h"
 #include "shrpx_config.h"
 #include "shrpx_http.h"
-#include "shrpx_worker_config.h"
 #include "shrpx_worker.h"
 #include "http2.h"
 #include "util.h"

src/shrpx_http_downstream_connection.cc

@@ -30,7 +30,7 @@
 #include "shrpx_config.h"
 #include "shrpx_error.h"
 #include "shrpx_http.h"
-#include "shrpx_worker_config.h"
+#include "shrpx_log_config.h"
 #include "shrpx_connect_blocker.h"
 #include "shrpx_downstream_connection_pool.h"
 #include "shrpx_worker.h"
@@ -358,7 +358,7 @@ int HttpDownstreamConnection::push_request_headers() {
   if (LOG_ENABLED(INFO)) {
     const char *hdrp;
     std::string nhdrs;
-    if (worker_config->errorlog_tty) {
+    if (log_config->errorlog_tty) {
       nhdrs = http::colorizeHeaders(hdrs.c_str());
       hdrp = nhdrs.c_str();
     } else {

src/shrpx_https_upstream.cc

@@ -34,7 +34,7 @@
 #include "shrpx_http.h"
 #include "shrpx_config.h"
 #include "shrpx_error.h"
-#include "shrpx_worker_config.h"
+#include "shrpx_log_config.h"
 #include "shrpx_worker.h"
 #include "http2.h"
 #include "util.h"
@@ -810,7 +810,7 @@ int HttpsUpstream::on_downstream_abort_request(Downstream *downstream,
 void HttpsUpstream::log_response_headers(const std::string &hdrs) const {
   const char *hdrp;
   std::string nhdrs;
-  if (worker_config->errorlog_tty) {
+  if (log_config->errorlog_tty) {
     nhdrs = http::colorizeHeaders(hdrs.c_str());
     hdrp = nhdrs.c_str();
   } else {

src/shrpx_log.cc

@@ -37,7 +37,6 @@
 
 #include "shrpx_config.h"
 #include "shrpx_downstream.h"
-#include "shrpx_worker_config.h"
 #include "util.h"
 #include "template.h"
 
@@ -100,10 +99,10 @@ Log::~Log() {
     return;
   }
 
-  auto wconf = worker_config;
+  auto lgconf = log_config;
 
   if (!log_enabled(severity_) ||
-      (wconf->errorlog_fd == -1 && !get_config()->errorlog_syslog)) {
+      (lgconf->errorlog_fd == -1 && !get_config()->errorlog_syslog)) {
     return;
   }
 
@@ -121,10 +120,10 @@ Log::~Log() {
   }
 
   char buf[4096];
-  auto tty = wconf->errorlog_tty;
+  auto tty = lgconf->errorlog_tty;
 
-  wconf->update_tstamp(std::chrono::system_clock::now());
-  auto &time_local = wconf->time_local_str;
+  lgconf->update_tstamp(std::chrono::system_clock::now());
+  auto &time_local = lgconf->time_local_str;
 
   if (severity_ == NOTICE) {
     rv = snprintf(buf, sizeof(buf), "%s PID%d [%s%s%s] %s\n",
@@ -145,7 +144,7 @@ Log::~Log() {
 
   auto nwrite = std::min(static_cast<size_t>(rv), sizeof(buf) - 1);
 
-  while (write(wconf->errorlog_fd, buf, nwrite) == -1 && errno == EINTR)
+  while (write(lgconf->errorlog_fd, buf, nwrite) == -1 && errno == EINTR)
     ;
 }
 
@@ -160,9 +159,9 @@ std::pair<OutputIterator, size_t> copy(const char *src, size_t avail,
 } // namespace
 
 void upstream_accesslog(const std::vector<LogFragment> &lfv, LogSpec *lgsp) {
-  auto wconf = worker_config;
+  auto lgconf = log_config;
 
-  if (wconf->accesslog_fd == -1 && !get_config()->accesslog_syslog) {
+  if (lgconf->accesslog_fd == -1 && !get_config()->accesslog_syslog) {
     return;
   }
 
@@ -173,9 +172,9 @@ void upstream_accesslog(const std::vector<LogFragment> &lfv, LogSpec *lgsp) {
   auto p = buf;
   auto avail = sizeof(buf) - 2;
 
-  wconf->update_tstamp(lgsp->time_now);
-  auto &time_local = wconf->time_local_str;
-  auto &time_iso8601 = wconf->time_iso8601_str;
+  lgconf->update_tstamp(lgsp->time_now);
+  auto &time_local = lgconf->time_local_str;
+  auto &time_iso8601 = lgconf->time_iso8601_str;
 
   for (auto &lf : lfv) {
     switch (lf.type) {
@@ -266,26 +265,26 @@ void upstream_accesslog(const std::vector<LogFragment> &lfv, LogSpec *lgsp) {
   *p++ = '\n';
 
   auto nwrite = p - buf;
-  while (write(wconf->accesslog_fd, buf, nwrite) == -1 && errno == EINTR)
+  while (write(lgconf->accesslog_fd, buf, nwrite) == -1 && errno == EINTR)
     ;
 }
 
 int reopen_log_files() {
   int res = 0;
 
-  auto wconf = worker_config;
+  auto lgconf = log_config;
 
-  if (wconf->accesslog_fd != -1) {
-    close(wconf->accesslog_fd);
-    wconf->accesslog_fd = -1;
+  if (lgconf->accesslog_fd != -1) {
+    close(lgconf->accesslog_fd);
+    lgconf->accesslog_fd = -1;
   }
 
   if (!get_config()->accesslog_syslog && get_config()->accesslog_file) {
 
-    wconf->accesslog_fd =
+    lgconf->accesslog_fd =
         util::reopen_log_file(get_config()->accesslog_file.get());
 
-    if (wconf->accesslog_fd == -1) {
+    if (lgconf->accesslog_fd == -1) {
       LOG(ERROR) << "Failed to open accesslog file "
                  << get_config()->accesslog_file.get();
       res = -1;
@@ -299,7 +298,7 @@ int reopen_log_files() {
     new_errorlog_fd = util::reopen_log_file(get_config()->errorlog_file.get());
 
     if (new_errorlog_fd == -1) {
-      if (wconf->errorlog_fd != -1) {
+      if (lgconf->errorlog_fd != -1) {
         LOG(ERROR) << "Failed to open errorlog file "
                    << get_config()->errorlog_file.get();
       } else {
@@ -311,15 +310,15 @@ int reopen_log_files() {
     }
   }
 
-  if (wconf->errorlog_fd != -1) {
-    close(wconf->errorlog_fd);
-    wconf->errorlog_fd = -1;
-    wconf->errorlog_tty = false;
+  if (lgconf->errorlog_fd != -1) {
+    close(lgconf->errorlog_fd);
+    lgconf->errorlog_fd = -1;
+    lgconf->errorlog_tty = false;
   }
 
   if (new_errorlog_fd != -1) {
-    wconf->errorlog_fd = new_errorlog_fd;
-    wconf->errorlog_tty = isatty(wconf->errorlog_fd);
+    lgconf->errorlog_fd = new_errorlog_fd;
+    lgconf->errorlog_tty = isatty(lgconf->errorlog_fd);
   }
 
   return res;

src/shrpx_log.h

@@ -34,6 +34,8 @@
 #include <vector>
 #include <chrono>
 
+#include "shrpx_log_config.h"
+
 namespace shrpx {
 
 class Downstream;
@@ -95,8 +97,8 @@ private:
   static int severity_thres_;
 };
 
-#define TTY_HTTP_HD (worker_config->errorlog_tty ? "\033[1;34m" : "")
-#define TTY_RST (worker_config->errorlog_tty ? "\033[0m" : "")
+#define TTY_HTTP_HD (log_config->errorlog_tty ? "\033[1;34m" : "")
+#define TTY_RST (log_config->errorlog_tty ? "\033[0m" : "")
 
 enum LogFragmentType {
   SHRPX_LOGF_NONE,

src/shrpx_log_config.cc

@@ -22,23 +22,23 @@
  * 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_worker_config.h"
+#include "shrpx_log_config.h"
 #include "util.h"
 
 using namespace nghttp2;
 
 namespace shrpx {
 
-WorkerConfig::WorkerConfig()
+LogConfig::LogConfig()
     : accesslog_fd(-1), errorlog_fd(-1), errorlog_tty(false) {}
 
 #ifndef NOTHREADS
 thread_local
 #endif // NOTHREADS
-    WorkerConfig *worker_config = new WorkerConfig();
+    LogConfig *log_config = new LogConfig();
 
 void
-WorkerConfig::update_tstamp(const std::chrono::system_clock::time_point &now) {
+LogConfig::update_tstamp(const std::chrono::system_clock::time_point &now) {
   auto t0 = std::chrono::system_clock::to_time_t(time_str_updated_);
   auto t1 = std::chrono::system_clock::to_time_t(now);
   if (t0 == t1) {

src/shrpx_log_config.h

@@ -22,8 +22,8 @@
  * 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_WORKER_CONFIG_H
-#define SHRPX_WORKER_CONFIG_H
+#ifndef SHRPX_LOG_CONFIG_H
+#define SHRPX_LOG_CONFIG_H
 
 #include "shrpx.h"
 
@@ -31,13 +31,7 @@
 
 namespace shrpx {
 
-namespace ssl {
-class CertLookupTree;
-} // namespace ssl
-
-struct TicketKeys;
-
-struct WorkerConfig {
+struct LogConfig {
   std::chrono::system_clock::time_point time_str_updated_;
   std::string time_local_str;
   std::string time_iso8601_str;
@@ -46,17 +40,18 @@ struct WorkerConfig {
   // true if errorlog_fd is referring to a terminal.
   bool errorlog_tty;
 
-  WorkerConfig();
+  LogConfig();
   void update_tstamp(const std::chrono::system_clock::time_point &now);
 };
 
-// We need WorkerConfig per thread
+// We need LogConfig per thread to avoid data race around opening file
+// descriptor for log files.
 extern
 #ifndef NOTHREADS
     thread_local
 #endif // NOTHREADS
-    WorkerConfig *worker_config;
+    LogConfig *log_config;
 
 } // namespace shrpx
 
-#endif // SHRPX_WORKER_CONFIG_H
+#endif // SHRPX_LOG_CONFIG_H

src/shrpx_spdy_upstream.cc

@@ -36,7 +36,6 @@
 #include "shrpx_downstream_connection.h"
 #include "shrpx_config.h"
 #include "shrpx_http.h"
-#include "shrpx_worker_config.h"
 #include "http2.h"
 #include "util.h"
 #include "template.h"

src/shrpx_worker.cc

@@ -32,7 +32,7 @@
 #include "shrpx_log.h"
 #include "shrpx_client_handler.h"
 #include "shrpx_http2_session.h"
-#include "shrpx_worker_config.h"
+#include "shrpx_log_config.h"
 #include "shrpx_connect_blocker.h"
 #include "util.h"
 #include "template.h"
@@ -137,8 +137,7 @@ void Worker::process_events() {
     }
     case RENEW_TICKET_KEYS:
       if (LOG_ENABLED(INFO)) {
-        WLOG(INFO, this) << "Renew ticket keys: worker_info(" << worker_config
-                         << ")";
+        WLOG(INFO, this) << "Renew ticket keys: worker(" << this << ")";
       }
 
       ticket_keys_ = wev.ticket_keys;
@@ -146,8 +145,7 @@ void Worker::process_events() {
       break;
     case REOPEN_LOG:
       if (LOG_ENABLED(INFO)) {
-        WLOG(INFO, this) << "Reopening log files: worker_info(" << worker_config
-                         << ")";
+        WLOG(INFO, this) << "Reopening log files: worker(" << this << ")";
       }
 
       reopen_log_files();