CRM-19798 fix memory leak on EntityTag.get & other calls in the DB (generic fix)

[eileenmcnaughton]

Overview

Fix memory leak that manifests (among other places) when calling api EntityTag.get

Before

Memory in use before & after 100 iterations of EntityTag.get

Start :'49,864,896'
End :'53,503,264'

After

Memory in use before & after 100 iterations of EntityTag.get

Start :'49,851,680'
End:'49,852,168'

Technical Details

The PR adds a class destructor to the DAO that frees the mysql resource. From testing it's fine if it has already been freed it gets most code paths (there is one that it does not - see below)

I dug fairly heavily into the way the DAO cache is currently being used. It builds up queries in the global
$_DB_DATAOBJECT['RESULTS'];
From my investigations these are used for iterating through a result set rather than caching the results of a query. ie. in the case of this function

function demonstrate() {
$dao = new CRM_Core_DAO_Domain();
$dao->find(TRUE)
return $dao->locales;
}
there will be an entry remaining in
$_DB_DATAOBJECT['RESULTS']
representing the mysql results - but it can only be used from that $dao. Once we have left the function the $dao will be destroyed but the object to retrieve the result set will remain in the global, unusable but memory hogging.

If the $dao is freed before it's time then $dao->fetch() will no longer retrieve results - potentially causing unpredictable errors (this is the risk of calling CRM_Core_DAO::freeResult()) as an outer loop may be disbanded. I removed these calls as they are no longer doing any good (& can do harm)

Path still not releasing memory:

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

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

Comments

This replaces #11615

We have been running #11615 in production for several months under load. In that time we observed one issue - the free result broke down when the DAO was cloned. I have updated this with a patch that replicates that issue and fixes it.

It should be noted the free result bug ALSO happens without this patch, but in obscure & unknown circumstances. It is certainly replicable by doing basically the same sequence as in the added test bue using CRM_Core_DAO::freeResult();

I submitted various versions of this change for CI & noticed no particular difference in the duration of the jenkins job. I think I saw more difference locally but did not do multiple tests on that. Definitely no worse! I was able to replicate the memory management issue in a test & submitted the test without the changes to see if that is replication on jenkins #11616

---

• CRM-19798: Memory leak in API3 EntityTag get operations

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@mlutfy FYI - you asked on previous version if we had prod tested -this version addresses the one (obscure) prod issue we hit

[eileenmcnaughton]

(note I revived this after a stackexchange question - it had been languishing on my to-do list as we have deployed it & upstreaming needed me to take the extra clone step)

[colemanw]

This looks safe, well-documented and well-tested. I'm going to merge it now since we're pre-beta for 5.4 and it will get some eyes on it before the release.