Fix memory leak

valbok · Mar 19, 2023 · ea375ef · ea375ef
1 parent 20bebf7
commit ea375ef
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 2 deletions.
diff --git a/src/Common/mysqlxx/Pool.cpp b/src/Common/mysqlxx/Pool.cpp
@@ -33,9 +33,12 @@ void Pool::Entry::incrementRefCount()
 {
     if (!data)
         return;
+
     /// First reference, initialize thread
     if (data->ref_count.fetch_add(1) == 0)
         mysql_thread_init();
+
+    chassert(!data->removed_from_pool);
 }
 
 
@@ -44,9 +47,20 @@ void Pool::Entry::decrementRefCount()
     if (!data)
         return;
 
-    /// We were the last user of this thread, deinitialize it
-    if (data->ref_count.fetch_sub(1) == 1)
+    const auto ref_count = data->ref_count.fetch_sub(1);
+    if (ref_count == 1)
+    {
+        /// We were the last user of this thread, deinitialize it
         mysql_thread_end();
+    }
+    else if (data->removed_from_pool)
+    {
+        /// data->ref_count == 0 in case we removed connection from pool (see Pool::removeConnection).
+        chassert(ref_count == 0);
+        /// In Pool::Entry::disconnect() we remove connection from the list of pool's connections.
+        /// So now we must deallocate the memory.
+        ::delete data;
+    }
 }
 
 
@@ -238,6 +252,7 @@ void Pool::removeConnection(Connection* connection)
             connection->ref_count = 0;
         }
         connections.remove(connection);
+        connection->removed_from_pool = true;
     }
 }
 

diff --git a/src/Common/mysqlxx/mysqlxx/Pool.h b/src/Common/mysqlxx/mysqlxx/Pool.h
@@ -41,6 +41,7 @@ class Pool final
         /// Ref count modified in constructor/descructor of Entry
         /// but also read in pool code.
         std::atomic<int> ref_count = 0;
+        std::atomic<bool> removed_from_pool = false;
     };
 
 public:

diff --git a/tests/integration/test_mysql_database_engine/test.py b/tests/integration/test_mysql_database_engine/test.py
@@ -999,3 +999,25 @@ def test_restart_server(started_cluster):
             clickhouse_node.restart_clickhouse()
             clickhouse_node.query_and_get_error("SHOW TABLES FROM test_restart")
         assert "test_table" in clickhouse_node.query("SHOW TABLES FROM test_restart")
+
+
+def test_memory_leak(started_cluster):
+    with contextlib.closing(
+        MySQLNodeInstance(
+            "root", "clickhouse", started_cluster.mysql_ip, started_cluster.mysql_port
+        )
+    ) as mysql_node:
+        mysql_node.query("DROP DATABASE IF EXISTS test_database")
+        mysql_node.query("CREATE DATABASE test_database DEFAULT CHARACTER SET 'utf8'")
+        mysql_node.query(
+            "CREATE TABLE `test_database`.`test_table` ( `id` int(11) NOT NULL, PRIMARY KEY (`id`) ) ENGINE=InnoDB;"
+        )
+
+        clickhouse_node.query("DROP DATABASE IF EXISTS test_database")
+        clickhouse_node.query(
+            "CREATE DATABASE test_database ENGINE = MySQL('mysql57:3306', 'test_database', 'root', 'clickhouse') SETTINGS connection_auto_close = 1"
+        )
+        clickhouse_node.query("SELECT count() FROM `test_database`.`test_table`")
+
+        clickhouse_node.query("DROP DATABASE test_database")
+        clickhouse_node.restart_clickhouse()