Fix a rare segfault happening when a pool is destroyed in the middle …

…of running conn cleanup

Summary:
Previously, I tried to fix a rare segfault that could happen when a pool is destroyed while connection cleanup routine traverses in a map and deleting MysqlPooledHolder pointer. This could happen because deleting MysqlPooledHolder pointer could decrease reference counter of pool to zero, leading to calling the pool destructor.

Previous attempt to fix this was done by keeping MysqlPooledHolder pointer while the cleanup is in progress, but this was not enough because MysqlPooledHolder only has a weak_ptr to pool (AsynConnectionPool).

In this diff, I try to keep shared_ptr<AsyncConnectionPool> so that the pool's destructor is never called while the cleanup is in progress. Also, I made the cleanup exit early if the pool is already destructed, which is identifiable when locking weak_ptr fails.

Reviewed By: jkedgar

Differential Revision: D35458447

fbshipit-source-id: 5bd06edd38304556375af0c89b1e8aa064e88802

squangle/mysql_client/AsyncConnectionPool.h

@@ -117,16 +117,22 @@ class TwoLevelCache {
   // Cleans up connections in level2_ (and level1_) cache, which meet the pred.
   template <typename Pred>
   void cleanup(Pred pred) {
-    // We keep one value to prevent its pool from being destructed in the middle
-    // of traversing the map
-    std::optional<Value> value;
+    // We keep shared_ptr here to prevent the pool from being destructed in the
+    // middle of traversing the map
+    std::shared_ptr<AsyncConnectionPool> pool;
     for (auto it1 = level1_.begin(); it1 != level1_.end();) {
       auto& list = it1->second;
       DCHECK(!list.empty());
       for (auto it2 = list.begin(); it2 != list.end();) {
         if (pred(*it2)) {
-          if (!value) {
-            value = std::move(*it2);
+          if (!pool) {
+            pool = (*it2)->getPoolPtr();
+            if (!pool) {
+              // the pool is already destroyed
+              LOG(DFATAL)
+                  << "pool was already destroyed while cleanup was in progress";
+              return;
+            }
           }
           it2 = list.erase(it2);
         } else {
@@ -408,6 +414,10 @@ class MysqlPooledHolder : public MysqlConnectionHolder {
     return pool_key_;
   }
 
+  std::shared_ptr<AsyncConnectionPool> getPoolPtr() const {
+    return weak_pool_.lock();
+  }
+
  private:
   void removeFromPool();