From: Peter van Dijk <peter.van.dijk@powerdns.com>
Date: Thu, 21 Feb 2019 20:38:01 +0000 (+0000)
Subject: plug mysql_thread_init memory leak, fixes #6231
X-Git-Tag: dnsdist-1.4.0-alpha1~47^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=863025cc2f7930b1249fcc5861fe7ff6f192ebfe;p=thirdparty%2Fpdns.git

plug mysql_thread_init memory leak, fixes #6231
---

diff --git a/docs/backends/generic-mysql.rst b/docs/backends/generic-mysql.rst
index c04070d58c..dc69801ffc 100644
--- a/docs/backends/generic-mysql.rst
+++ b/docs/backends/generic-mysql.rst
@@ -123,6 +123,15 @@ Use the InnoDB READ-COMMITTED transaction isolation level. Default: yes.
 The timeout in seconds for each attempt to read from, or write to the
 server. A value of 0 will disable the timeout. Default: 10
 
+.. _setting-gmysql-thread-closer:
+
+``gmysql-thread-closer``
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+Older versions (such as those shipped on RHEL 7) of the MySQL/MariaDB client libraries leak memory unless applications explicitly report the end of each thread to the library. Enabling ``gmysql-thread-closer`` tells PowerDNS to call ``mysql_thread_end()`` whenever a thread ends.
+
+Only enable this if you are certain you need to. For more discussion, see https://github.com/PowerDNS/pdns/issues/6231.
+
 Default Schema
 --------------
 
diff --git a/modules/gmysqlbackend/gmysqlbackend.cc b/modules/gmysqlbackend/gmysqlbackend.cc
index e3033abe3b..48707d3a95 100644
--- a/modules/gmysqlbackend/gmysqlbackend.cc
+++ b/modules/gmysqlbackend/gmysqlbackend.cc
@@ -59,7 +59,8 @@ void gMySQLBackend::reconnect()
                    getArg("password"),
                    getArg("group"),
                    mustDo("innodb-read-committed"),
-                   getArgAsNum("timeout")));
+                   getArgAsNum("timeout"),
+                   mustDo("thread-cleanup")));
 }
 
 class gMySQLFactory : public BackendFactory
@@ -78,6 +79,7 @@ public:
     declare(suffix,"group", "Database backend MySQL 'group' to connect as", "client");
     declare(suffix,"innodb-read-committed","Use InnoDB READ-COMMITTED transaction isolation level","yes");
     declare(suffix,"timeout", "The timeout in seconds for each attempt to read/write to the server", "10");
+    declare(suffix,"thread-cleanup","Explicitly call mysql_thread_end() when threads end","no");
 
     declare(suffix,"dnssec","Enable DNSSEC processing","no");
 
diff --git a/modules/gmysqlbackend/smysql.cc b/modules/gmysqlbackend/smysql.cc
index 1d851daa91..0b06257477 100644
--- a/modules/gmysqlbackend/smysql.cc
+++ b/modules/gmysqlbackend/smysql.cc
@@ -38,6 +38,32 @@
 typedef bool my_bool;
 #endif
 
+/*
+ * Older versions of the MySQL and MariaDB client leak memory
+ * because they expect the application to call mysql_thread_end()
+ * when a thread ends. This thread_local static object provides
+ * that closure, but only when the user has asked for it
+ * by setting gmysql-thread-cleanup.
+ * For more discussion, see https://github.com/PowerDNS/pdns/issues/6231
+ */
+class MySQLThreadCloser
+{
+public:
+  ~MySQLThreadCloser() {
+    if(d_enabled) {
+      mysql_thread_end();
+    }
+  }
+  void enable() {
+   d_enabled = true;
+  }
+
+private:
+  bool d_enabled = false;
+};
+
+static thread_local MySQLThreadCloser threadcloser;
+
 bool SMySQL::s_dolog;
 pthread_mutex_t SMySQL::s_myinitlock = PTHREAD_MUTEX_INITIALIZER;
 
@@ -419,6 +445,10 @@ void SMySQL::connect()
   int retry=1;
 
   Lock l(&s_myinitlock);
+  if (d_threadCleanup) {
+    threadcloser.enable();
+  }
+
   if (!mysql_init(&d_db))
     throw sPerrorException("Unable to initialize mysql driver");
 
@@ -467,8 +497,8 @@ void SMySQL::connect()
 }
 
 SMySQL::SMySQL(const string &database, const string &host, uint16_t port, const string &msocket, const string &user,
-               const string &password, const string &group, bool setIsolation, unsigned int timeout):
-  d_database(database), d_host(host), d_msocket(msocket), d_user(user), d_password(password), d_group(group), d_timeout(timeout), d_port(port), d_setIsolation(setIsolation)
+               const string &password, const string &group, bool setIsolation, unsigned int timeout, bool threadCleanup):
+  d_database(database), d_host(host), d_msocket(msocket), d_user(user), d_password(password), d_group(group), d_timeout(timeout), d_port(port), d_setIsolation(setIsolation), d_threadCleanup(threadCleanup)
 {
   connect();
 }
diff --git a/modules/gmysqlbackend/smysql.hh b/modules/gmysqlbackend/smysql.hh
index 7e31d7b564..7a33e8c529 100644
--- a/modules/gmysqlbackend/smysql.hh
+++ b/modules/gmysqlbackend/smysql.hh
@@ -32,7 +32,8 @@ public:
   SMySQL(const string &database, const string &host="", uint16_t port=0,
          const string &msocket="",const string &user="",
          const string &password="", const string &group="",
-         bool setIsolation=false, unsigned int timeout=10);
+         bool setIsolation=false, unsigned int timeout=10,
+         bool threadCleanup=false);
 
   ~SMySQL();
 
@@ -61,6 +62,7 @@ private:
   unsigned int d_timeout;
   uint16_t d_port;
   bool d_setIsolation;
+  bool d_threadCleanup;
 };
 
 #endif /* SSMYSQL_HH */