From: eileen <emcnaughton@wikimedia.org>
Date: Wed, 31 Jan 2018 04:53:49 +0000 (+1300)
Subject: CRM-19798 fix memory leak on EntityTag.get
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=ffcc1d113643a89dff6a7a7bf1fb3665de1cb751;p=civicrm-core.git

CRM-19798 fix memory leak on EntityTag.get

This is an alternate methodology using __destruct on the DAO object.

Note that I have found some places bypass this - ie.

```
   $rule = new CRM_ACL_BAO_ACL();
   $rule->query($query);
```

But I think that is a pretty clumsy construct & we should swap to
CRM_Core_DAO::executeQuery();
---

diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php
index 0149986863..f222e19594 100644
--- a/CRM/Contact/BAO/Contact.php
+++ b/CRM/Contact/BAO/Contact.php
@@ -1022,10 +1022,6 @@ WHERE     civicrm_contact.id = " . CRM_Utils_Type::escape($id, 'Integer');
       CRM_Utils_Hook::post('delete', $contactType, $contact->id, $contact);
     }
 
-    // also reset the DB_DO global array so we can reuse the memory
-    // http://issues.civicrm.org/jira/browse/CRM-4387
-    CRM_Core_DAO::freeResult();
-
     return TRUE;
   }
 
diff --git a/CRM/Contact/Import/Parser.php b/CRM/Contact/Import/Parser.php
index 552ec453f6..6a40dea23f 100644
--- a/CRM/Contact/Import/Parser.php
+++ b/CRM/Contact/Import/Parser.php
@@ -275,9 +275,6 @@ abstract class CRM_Contact_Import_Parser extends CRM_Import_Parser {
         break;
       }
 
-      // clean up memory from dao's
-      CRM_Core_DAO::freeResult();
-
       // see if we've hit our timeout yet
       /* if ( $the_thing_with_the_stuff ) {
       do_something( );
diff --git a/CRM/Core/BAO/RecurringEntity.php b/CRM/Core/BAO/RecurringEntity.php
index fb73a78cf2..a5ab757b7b 100644
--- a/CRM/Core/BAO/RecurringEntity.php
+++ b/CRM/Core/BAO/RecurringEntity.php
@@ -660,7 +660,6 @@ class CRM_Core_BAO_RecurringEntity extends CRM_Core_DAO_RecurringEntity {
         }
 
         $updateDAO = CRM_Core_DAO::cascadeUpdate($daoName, $obj->id, $entityID, $skipData);
-        CRM_Core_DAO::freeResult();
       }
       else {
         CRM_Core_Error::fatal("DAO Mapper missing for $entityTable.");
diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index 6df94eb19a..584f187e09 100644
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -48,6 +48,13 @@ require_once 'CRM/Core/I18n.php';
  */
 class CRM_Core_DAO extends DB_DataObject {
 
+  /**
+   * How many times has this instance been cloned.
+   *
+   * @var int
+   */
+  protected $resultCopies = 0;
+
   /**
    * @var null
    * @deprecated
@@ -119,6 +126,22 @@ class CRM_Core_DAO extends DB_DataObject {
     $this->__table = $this->getTableName();
   }
 
+  public function __clone() {
+    if (!empty($this->_DB_resultid)) {
+      $this->resultCopies++;
+    }
+  }
+
+  /**
+   * Class destructor.
+   */
+  public function __destruct() {
+    if ($this->resultCopies === 0) {
+      $this->free();
+    }
+    $this->resultCopies--;
+  }
+
   /**
    * Empty definition for virtual function.
    */
diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php
index e57e83d637..009f842c0a 100644
--- a/CRM/Dedupe/Merger.php
+++ b/CRM/Dedupe/Merger.php
@@ -1457,7 +1457,6 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
     $otherTree = CRM_Core_BAO_CustomGroup::getTree($main['contact_type'], NULL, $otherId, -1,
       CRM_Utils_Array::value('contact_sub_type', $other), NULL, TRUE, NULL, TRUE, $checkPermissions
     );
-    CRM_Core_DAO::freeResult();
 
     foreach ($otherTree as $gid => $group) {
       $foundField = FALSE;
@@ -2328,8 +2327,6 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
       // pair may have been flipped, so make sure we delete using both orders
       CRM_Core_BAO_PrevNextCache::deletePair($mainId, $otherId, $cacheKeyString, TRUE);
     }
-
-    CRM_Core_DAO::freeResult();
   }
 
 }
diff --git a/tests/phpunit/CRM/Core/DAOTest.php b/tests/phpunit/CRM/Core/DAOTest.php
index 399ecb6e40..9cf423c73a 100644
--- a/tests/phpunit/CRM/Core/DAOTest.php
+++ b/tests/phpunit/CRM/Core/DAOTest.php
@@ -433,4 +433,18 @@ class CRM_Core_DAOTest extends CiviUnitTestCase {
     }
   }
 
+  /**
+   * Test the DAO cloning method does not hit issues with freeing the result.
+   */
+  public function testCloneDAO() {
+    $dao = CRM_Core_DAO::executeQuery('SELECT * FROM civicrm_domain');
+    $i = 0;
+    while ($dao->fetch()) {
+      $i++;
+      $cloned = clone($dao);
+      unset($cloned);
+    }
+    $this->assertEquals(2, $i);
+  }
+
 }
diff --git a/tests/phpunit/api/v3/EntityTagTest.php b/tests/phpunit/api/v3/EntityTagTest.php
index beb8ae53fe..6541148860 100644
--- a/tests/phpunit/api/v3/EntityTagTest.php
+++ b/tests/phpunit/api/v3/EntityTagTest.php
@@ -139,6 +139,19 @@ class api_v3_EntityTagTest extends CiviUnitTestCase {
     $this->callAPIAndDocument('entity_tag', 'get', $paramsEntity, __FUNCTION__, __FILE__);
   }
 
+  /**
+   * Test memory usage does not escalate crazily.
+   */
+  public function testMemoryLeak() {
+    $start = memory_get_usage();
+    for ($i = 0; $i < 100; $i++) {
+      $this->callAPISuccess('EntityTag', 'get', []);
+      $memUsage = memory_get_usage();
+    }
+    $max = $start + 2000000;
+    $this->assertTrue($memUsage < $max, "mem usage ( $memUsage ) should be less than $max (start was $start) ");
+  }
+
   /**
    * Test tag can be added to a household.
    */