nghttpx: Simplify ticket handling between workers just using mutex

src/shrpx.cc

@@ -649,50 +649,39 @@ void renew_ticket_key_cb(struct ev_loop *loop, ev_timer *w, int revents) {
     auto &old_keys = old_ticket_keys->keys;
     auto &new_keys = ticket_keys->keys;
 
-    assert(old_keys.size() >= 2);
+    assert(!old_keys.empty());
 
-    new_keys.resize(std::min(13ul, old_keys.size() + 1));
+    new_keys.resize(std::min(12ul, old_keys.size() + 1));
-    std::copy_n(std::begin(old_keys), new_keys.size() - 2,
+    std::copy_n(std::begin(old_keys), new_keys.size() - 1,
                 std::begin(new_keys) + 1);
-    new_keys[0] = old_keys.back();
   } else {
-    ticket_keys->keys.resize(2);
+    ticket_keys->keys.resize(1);
-    if (generate_ticket_key(ticket_keys->keys[0]) != 0) {
-      if (LOG_ENABLED(INFO)) {
-        LOG(INFO) << "failed to generate ticket key";
-      }
-      conn_handler->set_ticket_keys(nullptr);
-      conn_handler->worker_renew_ticket_keys(nullptr);
-      return;
-    }
   }
 
-  auto &new_key = ticket_keys->keys.back();
+  auto &new_key = ticket_keys->keys[0];
 
   if (generate_ticket_key(new_key) != 0) {
     if (LOG_ENABLED(INFO)) {
       LOG(INFO) << "failed to generate ticket key";
     }
     conn_handler->set_ticket_keys(nullptr);
-    conn_handler->worker_renew_ticket_keys(nullptr);
+    conn_handler->set_ticket_keys_to_worker(nullptr);
     return;
   }
 
   if (LOG_ENABLED(INFO)) {
     LOG(INFO) << "ticket keys generation done";
-    assert(ticket_keys->keys.size() >= 2);
+    assert(ticket_keys->keys.size() >= 1);
-    LOG(INFO) << "enc+dec: "
+    LOG(INFO) << 0 << " enc+dec: "
               << util::format_hex(ticket_keys->keys[0].data.name);
-    for (size_t i = 1; i < ticket_keys->keys.size() - 1; ++i) {
+    for (size_t i = 1; i < ticket_keys->keys.size(); ++i) {
       auto &key = ticket_keys->keys[i];
-      LOG(INFO) << "dec: " << util::format_hex(key.data.name);
+      LOG(INFO) << i << " dec: " << util::format_hex(key.data.name);
     }
-    LOG(INFO) << "dec, next enc: "
-              << util::format_hex(ticket_keys->keys.back().data.name);
   }
 
   conn_handler->set_ticket_keys(ticket_keys);
-  conn_handler->worker_renew_ticket_keys(ticket_keys);
+  conn_handler->set_ticket_keys_to_worker(ticket_keys);
 }
 } // namespace
 

src/shrpx_connection_handler.cc

@@ -128,6 +128,13 @@ ConnectionHandler::~ConnectionHandler() {
   }
 }
 
+void ConnectionHandler::set_ticket_keys_to_worker(
+    const std::shared_ptr<TicketKeys> &ticket_keys) {
+  for (auto &worker : workers_) {
+    worker->set_ticket_keys(ticket_keys);
+  }
+}
+
 void ConnectionHandler::worker_reopen_log_files() {
   WorkerEvent wev{};
 
@@ -138,18 +145,6 @@ void ConnectionHandler::worker_reopen_log_files() {
   }
 }
 
-void ConnectionHandler::worker_renew_ticket_keys(
-    const std::shared_ptr<TicketKeys> &ticket_keys) {
-  WorkerEvent wev{};
-
-  wev.type = RENEW_TICKET_KEYS;
-  wev.ticket_keys = ticket_keys;
-
-  for (auto &worker : workers_) {
-    worker->send(wev);
-  }
-}
-
 void ConnectionHandler::create_single_worker() {
   auto cert_tree = ssl::create_cert_lookup_tree();
   auto sv_ssl_ctx = ssl::setup_server_ssl_context(all_ssl_ctx_, cert_tree);

src/shrpx_connection_handler.h

@@ -76,8 +76,9 @@ public:
   // Creates |num| Worker objects for multi threaded configuration.
   // The |num| must be strictly more than 1.
   void create_worker_thread(size_t num);
+  void
+  set_ticket_keys_to_worker(const std::shared_ptr<TicketKeys> &ticket_keys);
   void worker_reopen_log_files();
-  void worker_renew_ticket_keys(const std::shared_ptr<TicketKeys> &ticket_keys);
   void set_ticket_keys(std::shared_ptr<TicketKeys> ticket_keys);
   const std::shared_ptr<TicketKeys> &get_ticket_keys() const;
   struct ev_loop *get_loop() const;

src/shrpx_ssl.cc

@@ -188,7 +188,7 @@ int ticket_key_cb(SSL *ssl, unsigned char *key_name, unsigned char *iv,
                   EVP_CIPHER_CTX *ctx, HMAC_CTX *hctx, int enc) {
   auto handler = static_cast<ClientHandler *>(SSL_get_app_data(ssl));
   auto worker = handler->get_worker();
-  const auto &ticket_keys = worker->get_ticket_keys();
+  auto ticket_keys = worker->get_ticket_keys();
 
   if (!ticket_keys) {
     // No ticket keys available.

src/shrpx_worker.cc

@@ -172,12 +172,6 @@ void Worker::process_events() {
 
       break;
     }
-    case RENEW_TICKET_KEYS:
-      WLOG(NOTICE, this) << "Renew ticket keys: worker(" << this << ")";
-
-      ticket_keys_ = wev.ticket_keys;
-
-      break;
     case REOPEN_LOG:
       WLOG(NOTICE, this) << "Reopening log files: worker(" << this << ")";
 
@@ -206,11 +200,13 @@ void Worker::process_events() {
 
 ssl::CertLookupTree *Worker::get_cert_lookup_tree() const { return cert_tree_; }
 
-const std::shared_ptr<TicketKeys> &Worker::get_ticket_keys() const {
+std::shared_ptr<TicketKeys> Worker::get_ticket_keys() {
+  std::lock_guard<std::mutex> g(m_);
   return ticket_keys_;
 }
 
 void Worker::set_ticket_keys(std::shared_ptr<TicketKeys> ticket_keys) {
+  std::lock_guard<std::mutex> g(m_);
   ticket_keys_ = std::move(ticket_keys);
 }
 

src/shrpx_worker.h

@@ -75,7 +75,6 @@ enum WorkerEventType {
   NEW_CONNECTION = 0x01,
   REOPEN_LOG = 0x02,
   GRACEFUL_SHUTDOWN = 0x03,
-  RENEW_TICKET_KEYS = 0x04,
 };
 
 struct WorkerEvent {
@@ -100,8 +99,12 @@ public:
   void send(const WorkerEvent &event);
 
   ssl::CertLookupTree *get_cert_lookup_tree() const;
-  const std::shared_ptr<TicketKeys> &get_ticket_keys() const;
+
+  // These 2 functions make a lock m_ to get/set ticket keys
+  // atomically.
+  std::shared_ptr<TicketKeys> get_ticket_keys();
   void set_ticket_keys(std::shared_ptr<TicketKeys> ticket_keys);
+
   WorkerStat *get_worker_stat();
   DownstreamConnectionPool *get_dconn_pool();
   Http2Session *next_http2_session(size_t group);