Skip to content

Commit

Permalink
Fixing memory leak in ResultSet
Browse files Browse the repository at this point in the history
As a side effect of this fix, the $_query propery of ResultSet
was removed. The query will not appear anymore as part of the
__debugInfo() for a ResultSet anymore as a consequence of this.
  • Loading branch information
lorenzo committed Dec 8, 2015
1 parent 8634fb4 commit fa4a61f
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 12 deletions.
23 changes: 12 additions & 11 deletions src/ORM/ResultSet.php
Expand Up @@ -38,6 +38,7 @@ class ResultSet implements ResultSetInterface
* Original query from where results were generated
*
* @var Query
* @deprecated 3.1.6 Due to a memory leak, this property cannot be used anymore
*/
protected $_query;

Expand Down Expand Up @@ -169,16 +170,15 @@ class ResultSet implements ResultSetInterface
public function __construct($query, $statement)
{
$repository = $query->repository();
$this->_query = $query;
$this->_statement = $statement;
$this->_driver = $this->_query->connection()->driver();
$this->_defaultTable = $this->_query->repository();
$this->_calculateAssociationMap();
$this->_hydrate = $this->_query->hydrate();
$this->_driver = $query->connection()->driver();
$this->_defaultTable = $query->repository();
$this->_calculateAssociationMap($query);
$this->_hydrate = $query->hydrate();
$this->_entityClass = $repository->entityClass();
$this->_useBuffering = $query->bufferResults();
$this->_defaultAlias = $this->_defaultTable->alias();
$this->_calculateColumnMap();
$this->_calculateColumnMap($query);
$this->_calculateTypeMap();

if ($this->_useBuffering) {
Expand Down Expand Up @@ -347,11 +347,12 @@ public function count()
* Calculates the list of associations that should get eager loaded
* when fetching each record
*
* @param \Cake\ORM\Query $query The query from where to derive the associations
* @return void
*/
protected function _calculateAssociationMap()
protected function _calculateAssociationMap($query)
{
$map = $this->_query->eagerLoader()->associationsMap($this->_defaultTable);
$map = $query->eagerLoader()->associationsMap($this->_defaultTable);
$this->_matchingMap = (new Collection($map))
->match(['matching' => true])
->indexBy('alias')
Expand All @@ -367,12 +368,13 @@ protected function _calculateAssociationMap()
* Creates a map of row keys out of the query select clause that can be
* used to hydrate nested result sets more quickly.
*
* @param \Cake\ORM\Query $query The query from where to derive the column map
* @return void
*/
protected function _calculateColumnMap()
protected function _calculateColumnMap($query)
{
$map = [];
foreach ($this->_query->clause('select') as $key => $field) {
foreach ($query->clause('select') as $key => $field) {
$key = trim($key, '"`[]');
if (strpos($key, '__') > 0) {
$parts = explode('__', $key, 2);
Expand Down Expand Up @@ -622,7 +624,6 @@ protected function _castValues($alias, $values)
public function __debugInfo()
{
return [
'query' => $this->_query,
'items' => $this->toArray(),
];
}
Expand Down
25 changes: 25 additions & 0 deletions tests/TestCase/ORM/QueryRegressionTest.php
Expand Up @@ -1181,4 +1181,29 @@ public function testEagerLoadOrderAndSubquery()
$result = $query->all();
$this->assertCount(3, $result);
}

/**
* Tests that decorating the results does not result in a memory leak
*
* @return void
*/
public function testFormatResultsMemory()
{
$table = TableRegistry::get('Articles');
$table->belongsTo('Authors');
$table->belongsToMany('Tags');
gc_collect_cycles();
$memory = memory_get_usage() / 1024 / 1024;
foreach (range(1, 3) as $time) {
$table->find()
->contain(['Authors', 'Tags'])
->formatResults(function ($results) {
return $results;
})
->all();
}
gc_collect_cycles();
$endMemory = memory_get_usage() / 1024 / 1024;
$this->assertWithinRange($endMemory, $memory, 1.25, 'Memory leak in ResultSet');
}
}
1 change: 0 additions & 1 deletion tests/TestCase/ORM/ResultSetTest.php
Expand Up @@ -260,7 +260,6 @@ public function testDebugInfo()
$query = $this->table->find('all');
$results = $query->all();
$expected = [
'query' => $query,
'items' => $results->toArray()
];
$this->assertSame($expected, $results->__debugInfo());
Expand Down

0 comments on commit fa4a61f

Please sign in to comment.