Skip to content

Commit

Permalink
Merge pull request #12276 from eileenmcnaughton/memory_more
Browse files Browse the repository at this point in the history
CRM-19798 fix memory leak on EntityTag.get & other calls in the DB (generic fix)
  • Loading branch information
colemanw authored Jun 12, 2018
2 parents d3c1740 + ffcc1d1 commit dc080b3
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 11 deletions.
4 changes: 0 additions & 4 deletions CRM/Contact/BAO/Contact.php
Original file line number Diff line number Diff line change
Expand Up @@ -1022,10 +1022,6 @@ public static function deleteContact($id, $restore = FALSE, $skipUndelete = FALS
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;
}

Expand Down
3 changes: 0 additions & 3 deletions CRM/Contact/Import/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,6 @@ public function run(
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( );
Expand Down
1 change: 0 additions & 1 deletion CRM/Core/BAO/RecurringEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,6 @@ static public function triggerUpdate($event) {
}

$updateDAO = CRM_Core_DAO::cascadeUpdate($daoName, $obj->id, $entityID, $skipData);
CRM_Core_DAO::freeResult();
}
else {
CRM_Core_Error::fatal("DAO Mapper missing for $entityTable.");
Expand Down
23 changes: 23 additions & 0 deletions CRM/Core/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@
*/
class CRM_Core_DAO extends DB_DataObject {

/**
* How many times has this instance been cloned.
*
* @var int
*/
protected $resultCopies = 0;

/**
* @var null
* @deprecated
Expand Down Expand Up @@ -119,6 +126,22 @@ public function __construct() {
$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.
*/
Expand Down
3 changes: 0 additions & 3 deletions CRM/Dedupe/Merger.php
Original file line number Diff line number Diff line change
Expand Up @@ -1457,7 +1457,6 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio
$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;
Expand Down Expand Up @@ -2328,8 +2327,6 @@ protected static function dedupePair(&$migrationInfo, &$resultStats, &$deletedCo
// 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();
}

}
14 changes: 14 additions & 0 deletions tests/phpunit/CRM/Core/DAOTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -433,4 +433,18 @@ public function testFieldSerialization($method, $sampleData) {
}
}

/**
* 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);
}

}
13 changes: 13 additions & 0 deletions tests/phpunit/api/v3/EntityTagTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,19 @@ public function testIndividualEntityTagGet() {
$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.
*/
Expand Down

0 comments on commit dc080b3

Please sign in to comment.