From 04bd25d468aee33c03555146cf456505a0cfa9d2 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 23 Jul 2015 23:13:29 +0900 Subject: [PATCH] nghttpx: Simplify ticket handling between workers just using mutex --- src/shrpx.cc | 33 +++++++++++---------------------- src/shrpx_connection_handler.cc | 19 +++++++------------ src/shrpx_connection_handler.h | 3 ++- src/shrpx_ssl.cc | 2 +- src/shrpx_worker.cc | 10 +++------- src/shrpx_worker.h | 7 +++++-- 6 files changed, 29 insertions(+), 45 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 8b171ae1..aa97762e 100644 --- a/src/shrpx.cc +++ b/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)); - std::copy_n(std::begin(old_keys), new_keys.size() - 2, + new_keys.resize(std::min(12ul, old_keys.size() + 1)); + 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); - 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; - } + ticket_keys->keys.resize(1); } - 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); - LOG(INFO) << "enc+dec: " + assert(ticket_keys->keys.size() >= 1); + 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 diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index 91b477d2..64260e9d 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -128,6 +128,13 @@ ConnectionHandler::~ConnectionHandler() { } } +void ConnectionHandler::set_ticket_keys_to_worker( + const std::shared_ptr &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 &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); diff --git a/src/shrpx_connection_handler.h b/src/shrpx_connection_handler.h index 5b17fef2..c6488854 100644 --- a/src/shrpx_connection_handler.h +++ b/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 &ticket_keys); void worker_reopen_log_files(); - void worker_renew_ticket_keys(const std::shared_ptr &ticket_keys); void set_ticket_keys(std::shared_ptr ticket_keys); const std::shared_ptr &get_ticket_keys() const; struct ev_loop *get_loop() const; diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index ea5a1cc1..2d4635cd 100644 --- a/src/shrpx_ssl.cc +++ b/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(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. diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc index ecf55b51..449d4c48 100644 --- a/src/shrpx_worker.cc +++ b/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 &Worker::get_ticket_keys() const { +std::shared_ptr Worker::get_ticket_keys() { + std::lock_guard g(m_); return ticket_keys_; } void Worker::set_ticket_keys(std::shared_ptr ticket_keys) { + std::lock_guard g(m_); ticket_keys_ = std::move(ticket_keys); } diff --git a/src/shrpx_worker.h b/src/shrpx_worker.h index b5f820b4..1ad7e3b1 100644 --- a/src/shrpx_worker.h +++ b/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 &get_ticket_keys() const; + + // These 2 functions make a lock m_ to get/set ticket keys + // atomically. + std::shared_ptr get_ticket_keys(); void set_ticket_keys(std::shared_ptr ticket_keys); + WorkerStat *get_worker_stat(); DownstreamConnectionPool *get_dconn_pool(); Http2Session *next_http2_session(size_t group);