CRM-19798 fix memory leak on EntityTag.get
authoreileen <emcnaughton@wikimedia.org>
Wed, 31 Jan 2018 04:53:49 +0000 (17:53 +1300)
committereileen <emcnaughton@wikimedia.org>
Wed, 6 Jun 2018 21:25:56 +0000 (09:25 +1200)
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();

CRM/Contact/BAO/Contact.php
CRM/Contact/Import/Parser.php
CRM/Core/BAO/RecurringEntity.php
CRM/Core/DAO.php
CRM/Dedupe/Merger.php
tests/phpunit/CRM/Core/DAOTest.php
tests/phpunit/api/v3/EntityTagTest.php

index 01499868630a2a9a0aa678be672e3f3a20b261da..f222e19594a8f976f2e5586bcbbab2f45aa45af4 100644 (file)
@@ -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;
   }
 
index 552ec453f618997be0b1b739aa90fe809b707614..6a40dea23ff94e32898bbea78b99aa9e642ce715 100644 (file)
@@ -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( );
index fb73a78cf2cf1c21296de29ec276a7d32ceec975..a5ab757b7b8cecd8eaadb42abf84c7463f5cddc5 100644 (file)
@@ -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.");
index 6df94eb19ad1694f07d99ca3eae790d7a825cb44..584f187e09bbe7681643a6a2f8481b9ad6f00997 100644 (file)
@@ -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.
    */
index e57e83d6373cb4857758c7ca64df61041ae1a9d4..009f842c0aadb6274db1eeca80ba6a4350e410c0 100644 (file)
@@ -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();
   }
 
 }
index 399ecb6e40ba0e4755eba35037fd85bcf6456003..9cf423c73a28f0b788c81abe02865550f58e9546 100644 (file)
@@ -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);
+  }
+
 }
index beb8ae53fe017302b55d93dfaaa0d633292ebbe0..6541148860bf26f7db24a2ba5b3fba009e050eb7 100644 (file)
@@ -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.
    */