Use shared_ptr for ssl context objects

Summary:
The upstream RCU lock implementation can lead to situations where the `ALTER INSTANCE RELOAD TLS` spins cpu at 100% for a long time.

The solution to this is to use refcounting provided by `shared_ptr`.

Fixes https://bugs.mysql.com/bug.php?id=107567

Reviewed By: hermanlee

Differential Revision: D37188540

fbshipit-source-id: 37561bdcef56ae0cc1abbe63369baa7180938254

Author: lth

sql/ssl_acceptor_context_operator.cc

@@ -20,6 +20,7 @@
    along with this program; if not, write to the Free Software
    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA */
 
+#include "mutex_lock.h"
 #include "my_dbug.h"                                /* DBUG_ASSERT */
 #include "mysql/components/services/log_builtins.h" /* LogErr */
 #include "mysql/status_var.h"                       /* SHOW_VAR */
@@ -32,19 +33,19 @@ Ssl_acceptor_context_container *mysql_main;
 Ssl_acceptor_context_container *mysql_admin;
 
 Ssl_acceptor_context_container::Ssl_acceptor_context_container(
-    Ssl_acceptor_context_data *data)
-    : lock_(nullptr) {
-  lock_ = new Ssl_acceptor_context_data_lock(data);
+    std::shared_ptr<Ssl_acceptor_context_data> &&data)
+    : data_(data) {
+  mysql_mutex_init(PSI_NOT_INSTRUMENTED, &mutex_, MY_MUTEX_INIT_FAST);
 }
 
 Ssl_acceptor_context_container ::~Ssl_acceptor_context_container() {
-  if (lock_ != nullptr) delete lock_;
-  lock_ = nullptr;
+  mysql_mutex_destroy(&mutex_);
 }
 
 void Ssl_acceptor_context_container::switch_data(
-    Ssl_acceptor_context_data *new_data) {
-  if (lock_ != nullptr) lock_->write_wait_and_delete(new_data);
+    std::shared_ptr<Ssl_acceptor_context_data> &&new_data) {
+  MUTEX_LOCK(lock, &mutex_);
+  data_ = new_data;
 }
 
 bool TLS_channel::singleton_init(Ssl_acceptor_context_container **out,
@@ -67,10 +68,10 @@ bool TLS_channel::singleton_init(Ssl_acceptor_context_container **out,
     in auto-generation, bad key material explicitly specified or
     auto-generation disabled explcitly while SSL is still on.
   */
-  Ssl_acceptor_context_data *news =
-      new Ssl_acceptor_context_data(channel, use_ssl_arg, callbacks);
+  auto news = std::make_shared<Ssl_acceptor_context_data>(channel, use_ssl_arg,
+                                                          callbacks);
   Ssl_acceptor_context_container *new_container =
-      new Ssl_acceptor_context_container(news);
+      new Ssl_acceptor_context_container(std::move(news));
   if (news == nullptr || new_container == nullptr) {
     LogErr(WARNING_LEVEL, ER_SSL_LIBRARY_ERROR,
            "Error initializing the SSL context system structure");
@@ -101,29 +102,35 @@ void TLS_channel::singleton_flush(Ssl_acceptor_context_container *container,
                                   Ssl_init_callback *callbacks,
                                   enum enum_ssl_init_error *error, bool force) {
   if (container == nullptr) return;
-  Ssl_acceptor_context_data *news =
-      new Ssl_acceptor_context_data(channel, true, callbacks, false, error);
+  auto news = std::make_shared<Ssl_acceptor_context_data>(
+      channel, true, callbacks, false, error);
   if (*error != SSL_INITERR_NOERROR && !force) {
-    delete news;
     return;
   }
-  (void)container->switch_data(news);
+  (void)container->switch_data(std::move(news));
   return;
 }
 
+Lock_and_access_ssl_acceptor_context::Lock_and_access_ssl_acceptor_context(
+    Ssl_acceptor_context_container *context) {
+  // Take a reference
+  MUTEX_LOCK(lock, &context->mutex_);
+  data_ = context->data_;
+}
+
 std::string Lock_and_access_ssl_acceptor_context::show_property(
     Ssl_acceptor_context_property_type property_type) {
-  const Ssl_acceptor_context_data *data = read_lock_;
+  const Ssl_acceptor_context_data *data = data_.get();
   return (data != nullptr ? data->show_property(property_type) : std::string{});
 }
 
 std::string Lock_and_access_ssl_acceptor_context::channel_name() {
-  const Ssl_acceptor_context_data *data = read_lock_;
+  const Ssl_acceptor_context_data *data = data_.get();
   return (data != nullptr ? data->channel_name() : std::string{});
 }
 
 bool Lock_and_access_ssl_acceptor_context::have_ssl() {
-  const Ssl_acceptor_context_data *data = read_lock_;
+  const Ssl_acceptor_context_data *data = data_.get();
   return (data != nullptr ? data->have_ssl() : false);
 }
 

sql/ssl_acceptor_context_operator.h

@@ -23,7 +23,10 @@
 #ifndef SSL_ACCEPTOR_CONTEXT_OPERATOR
 #define SSL_ACCEPTOR_CONTEXT_OPERATOR
 
+#include <memory>
+
 #include <my_rcu_lock.h>                   /* MyRcuLock */
+#include <mysql/psi/mysql_mutex.h>
 #include "sql/ssl_acceptor_context_data.h" /** Ssl_acceptor_context_data */
 
 /* Types of supported contexts */
@@ -39,13 +42,13 @@ class TLS_channel;
 /** TLS context access protector */
 class Ssl_acceptor_context_container {
  protected:
-  Ssl_acceptor_context_container(Ssl_acceptor_context_data *data);
+  Ssl_acceptor_context_container(
+      std::shared_ptr<Ssl_acceptor_context_data> &&data);
   ~Ssl_acceptor_context_container();
-  void switch_data(Ssl_acceptor_context_data *new_data);
-
-  using Ssl_acceptor_context_data_lock = MyRcuLock<Ssl_acceptor_context_data>;
+  void switch_data(std::shared_ptr<Ssl_acceptor_context_data> &&new_data);
 
-  Ssl_acceptor_context_data_lock *lock_;
+  std::shared_ptr<Ssl_acceptor_context_data> data_;
+  mysql_mutex_t mutex_;
 
   /* F.R.I.E.N.D.S. */
   friend class Lock_and_access_ssl_acceptor_context;
@@ -103,37 +106,36 @@ using Ssl_acceptor_context_data_lock = MyRcuLock<Ssl_acceptor_context_data>;
 /** TLS context access wrapper for ease of use */
 class Lock_and_access_ssl_acceptor_context {
  public:
-  Lock_and_access_ssl_acceptor_context(Ssl_acceptor_context_container *context)
-      : read_lock_(context->lock_) {}
+  Lock_and_access_ssl_acceptor_context(Ssl_acceptor_context_container *context);
   ~Lock_and_access_ssl_acceptor_context() {}
 
   /** Access protected @ref Ssl_acceptor_context_data */
   operator const Ssl_acceptor_context_data *() {
-    const Ssl_acceptor_context_data *c = read_lock_;
+    const Ssl_acceptor_context_data *c = data_.get();
     return c;
   }
 
   /**
     Access to the SSL_CTX from the protected @ref Ssl_acceptor_context_data
   */
   operator SSL_CTX *() {
-    const Ssl_acceptor_context_data *c = read_lock_;
+    const Ssl_acceptor_context_data *c = data_.get();
     return c->ssl_acceptor_fd_->ssl_context;
   }
 
   /**
     Access to the SSL from the protected @ref Ssl_acceptor_context_data
   */
   operator SSL *() {
-    const Ssl_acceptor_context_data *c = read_lock_;
+    const Ssl_acceptor_context_data *c = data_.get();
     return c->acceptor_;
   }
 
   /**
     Access to st_VioSSLFd from the protected @ref Ssl_acceptor_context_data
   */
   operator struct st_VioSSLFd *() {
-    const Ssl_acceptor_context_data *c = read_lock_;
+    const Ssl_acceptor_context_data *c = data_.get();
     return c->ssl_acceptor_fd_;
   }
 
@@ -163,8 +165,7 @@ class Lock_and_access_ssl_acceptor_context {
   bool have_ssl();
 
  private:
-  /** Read lock over TLS context */
-  Ssl_acceptor_context_data_lock::ReadLock read_lock_;
+  std::shared_ptr<Ssl_acceptor_context_data> data_;
 };
 
 bool have_ssl();