Skip to content

Commit

Permalink
[BUGFIX beta] prevent calls to store.query leaking
Browse files Browse the repository at this point in the history
Although the result from `store.query` is intended to never be re-used
as we delegate to the server for the truth of a query, the user still
needs an escape hatch to destroy these AdapterPopulatedRecordArrays
created by query in order to prevent excessive memory.

Before, calling `destroy` on an AdapterPopulatedRecordArray (the type
returned from store.query), the logic in
RecordArrayManager#unregisterRecordArray did not account for adapter
populated record arrays resulting in a leak.

This commit changes the logic to search for the array in the
RecordArrayManager's AdapterPopulatedRecordArray collection when
unregistering a record array.

Fixes #4041

(cherry picked from commit 0eedecb)
  • Loading branch information
Stanley Stuart authored and bmac committed Jan 11, 2016
1 parent fedf581 commit 2227edd
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 10 deletions.
36 changes: 26 additions & 10 deletions addon/-private/system/record-array-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,17 +278,22 @@ export default Ember.Object.extend({
var typeClass = array.type;

// unregister filtered record array
var recordArrays = this.filteredRecordArrays.get(typeClass);
var index = recordArrays.indexOf(array);
if (index !== -1) {
recordArrays.splice(index, 1);

// unregister live record array
} else if (this.liveRecordArrays.has(typeClass)) {
var liveRecordArrayForType = this.liveRecordArrayFor(typeClass);
if (array === liveRecordArrayForType) {
this.liveRecordArrays.delete(typeClass);
const recordArrays = this.filteredRecordArrays.get(typeClass);
const removedFromFiltered = remove(recordArrays, array);

// remove from adapter populated record array
const removedFromAdapterPopulated = remove(this._adapterPopulatedRecordArrays, array);

if (!removedFromFiltered && !removedFromAdapterPopulated) {

// unregister live record array
if (this.liveRecordArrays.has(typeClass)) {
var liveRecordArrayForType = this.liveRecordArrayFor(typeClass);
if (array === liveRecordArrayForType) {
this.liveRecordArrays.delete(typeClass);
}
}

}
},

Expand All @@ -315,3 +320,14 @@ function flatten(list) {

return result;
}

function remove(array, item) {
const index = array.indexOf(item);

if (index !== -1) {
array.splice(index, 1);
return true;
}

return false;
}
24 changes: 24 additions & 0 deletions tests/integration/record-array-manager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,27 @@ test("Should not filter a store.peekAll() array when a record property is change
assert.equal(updateFilterRecordArray.called.length, 0);

});

test('#GH-4041 store#query AdapterPopulatedRecordArrays are removed from their managers instead of retained when #destroy is called', function(assert) {
run(() => {
store.push({
data: {
type: 'car',
id: '1',
attributes: {
make: 'Honda',
model: 'fit'
}
}
});
});
const query = {};

var adapterPopulated = manager.createAdapterPopulatedRecordArray(Car, query);

run(() => {
adapterPopulated.destroy();
});

assert.equal(manager._adapterPopulatedRecordArrays.length, 0);
});

0 comments on commit 2227edd

Please sign in to comment.