Skip to content

Commit

Permalink
Merge pull request #16584 from emberjs/mem-leak
Browse files Browse the repository at this point in the history
[BUGFIX] Memory issues
  • Loading branch information
rwjblue authored May 2, 2018
2 parents 3d91206 + 9e39aec commit 7d494bb
Show file tree
Hide file tree
Showing 23 changed files with 138 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ moduleFor(
setDebugFunction('debug', originalDebug);
if (appInstance) {
run(appInstance, 'destroy');
appInstance = null;
}

if (application) {
run(application, 'destroy');
application = null;
}

document.getElementById('qunit-fixture').innerHTML = '';
Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/application/tests/application_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ moduleFor(
[`@test acts like a namespace`](assert) {
let lookup = (context.lookup = {});

lookup.TestApp = this.runTask(() => this.createApplication());
lookup.TestApp = this.application = this.runTask(() => this.createApplication());

setNamespaceSearchDisabled(false);
let Foo = (this.application.Foo = EmberObject.extend());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ moduleFor(
}

teardown() {
super.teardown();
run(application, 'destroy');
application = undefined;
registry = undefined;
}

['@test normalization'](assert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ moduleFor(
}

teardown() {
super.teardown();
run(application, 'destroy');
application = locator = null;
registry = application = locator = null;
context.lookup = originalLookup;
}

Expand Down
1 change: 1 addition & 0 deletions packages/@ember/application/tests/readiness_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ moduleFor(
teardown() {
if (application) {
run(() => application.destroy());
jQuery = readyCallbacks = domReady = Application = application = undefined;
}
}

Expand Down
2 changes: 2 additions & 0 deletions packages/@ember/engine/tests/engine_initializers_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ moduleFor(
run(() => {
if (myEngineInstance) {
myEngineInstance.destroy();
myEngineInstance = null;
}

if (myEngine) {
myEngine.destroy();
myEngine = null;
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ moduleFor(
'Engine instance initializers',
class extends TestCase {
teardown() {
super.teardown();
run(() => {
if (myEngineInstance) {
myEngineInstance.destroy();
Expand All @@ -31,6 +32,7 @@ moduleFor(
myEngine.destroy();
}
});
MyEngine = myEngine = myEngineInstance = undefined;
}

["@test initializers require proper 'name' and 'initialize' properties"]() {
Expand Down
2 changes: 2 additions & 0 deletions packages/@ember/engine/tests/engine_instance_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ moduleFor(
teardown() {
if (engineInstance) {
run(engineInstance, 'destroy');
engineInstance = undefined;
}

if (engine) {
run(engine, 'destroy');
engine = undefined;
}
}

Expand Down
10 changes: 5 additions & 5 deletions packages/@ember/engine/tests/engine_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,28 @@ import {

let engine;
let originalLookup = context.lookup;
let lookup;

moduleFor(
'Engine',
class extends TestCase {
constructor() {
super();

lookup = context.lookup = {};
engine = run(() => Engine.create());
run(() => {
engine = Engine.create();
context.lookup = { TestEngine: engine };
});
}

teardown() {
context.lookup = originalLookup;
if (engine) {
run(engine, 'destroy');
engine = null;
}
}

['@test acts like a namespace'](assert) {
engine = run(() => (lookup.TestEngine = Engine.create()));

engine.Foo = EmberObject.extend();
assert.equal(
engine.Foo.toString(),
Expand Down
5 changes: 5 additions & 0 deletions packages/ember-extension-support/tests/data_adapter_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ const DataAdapter = EmberDataAdapter.extend({
moduleFor(
'Data Adapter',
class extends ApplicationTestCase {
teardown() {
super.teardown();
adapter = undefined;
}

['@test Model types added'](assert) {
this.add(
'data-adapter:main',
Expand Down
9 changes: 5 additions & 4 deletions packages/ember-routing/tests/location/auto_location_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function mockBrowserHistory(overrides, assert) {
}

function createLocation(location, history) {
let owner = buildOwner();
owner = buildOwner();

owner.register('location:history', HistoryLocation);
owner.register('location:hash', HashLocation);
Expand All @@ -56,14 +56,15 @@ function createLocation(location, history) {
return autolocation;
}

let location;
let location, owner;

moduleFor(
'AutoLocation',
class extends AbstractTestCase {
teardown() {
if (location) {
run(location, 'destroy');
if (owner) {
run(owner, 'destroy');
owner = location = undefined;
}
}

Expand Down
21 changes: 20 additions & 1 deletion packages/ember-routing/tests/system/route_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ moduleFor(
}

teardown() {
super.teardown();
runDestroy(route);
route = routeOne = routeTwo = lookupHash = undefined;
}

['@test default store utilizes the container to acquire the model factory'](assert) {
Expand Down Expand Up @@ -48,12 +50,15 @@ moduleFor(
},
};

setOwner(route, buildOwner(ownerOptions));
let owner = buildOwner(ownerOptions);
setOwner(route, owner);

route.set('_qp', null);

assert.equal(route.model({ post_id: 1 }), post);
assert.equal(route.findModel('post', 1), post, '#findModel returns the correct post');

runDestroy(owner);
}

["@test 'store' can be injected by data persistence frameworks"](assert) {
Expand Down Expand Up @@ -84,6 +89,8 @@ moduleFor(

assert.equal(route.model({ post_id: 1 }), post, '#model returns the correct post');
assert.equal(route.findModel('post', 1), post, '#findModel returns the correct post');

runDestroy(owner);
}

["@test assert if 'store.find' method is not found"]() {
Expand All @@ -100,6 +107,8 @@ moduleFor(
expectAssertion(function() {
route.findModel('post', 1);
}, 'Post has no method `find`.');

runDestroy(owner);
}

['@test asserts if model class is not found']() {
Expand All @@ -113,6 +122,8 @@ moduleFor(
expectAssertion(function() {
route.model({ post_id: 1 });
}, /You used the dynamic segment post_id in your route undefined, but <Ember.Object:ember\d+>.Post did not exist and you did not override your route\'s `model` hook./);

runDestroy(owner);
}

["@test 'store' does not need to be injected"](assert) {
Expand All @@ -129,6 +140,8 @@ moduleFor(
});

assert.ok(true, 'no error was raised');

runDestroy(owner);
}

["@test modelFor doesn't require the router"](assert) {
Expand All @@ -144,6 +157,8 @@ moduleFor(
owner.register('route:foo', FooRoute);

assert.strictEqual(route.modelFor('foo'), foo);

runDestroy(owner);
}

['@test .send just calls an action if the router is absent'](assert) {
Expand All @@ -167,6 +182,8 @@ moduleFor(
assert.equal(true, route.send('returnsTrue', 1, 2));
assert.equal(false, route.send('returnsFalse'));
assert.equal(undefined, route.send('nonexistent', 1, 2, 3));

runDestroy(route);
}

['@test .send just calls an action if the routers internal router property is absent'](assert) {
Expand All @@ -191,6 +208,8 @@ moduleFor(
assert.equal(true, route.send('returnsTrue', 1, 2));
assert.equal(false, route.send('returnsFalse'));
assert.equal(undefined, route.send('nonexistent', 1, 2, 3));

runDestroy(route);
}

['@test .send asserts if called on a destroyed route']() {
Expand Down
47 changes: 29 additions & 18 deletions packages/ember-runtime/tests/system/namespace/base_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ moduleFor(
}

['@test Namespace should be duck typed'](assert) {
assert.ok(get(Namespace.create(), 'isNamespace'), 'isNamespace property is true');
let namespace = Namespace.create();
try {
assert.ok(get(namespace, 'isNamespace'), 'isNamespace property is true');
} finally {
run(namespace, 'destroy');
}
}

['@test Namespace is found and named'](assert) {
Expand Down Expand Up @@ -90,23 +95,27 @@ moduleFor(
name: 'NamespaceA',
});

let nsB = (lookup.NamespaceB = Namespace.create({
name: 'CustomNamespaceB',
}));

nsA.Foo = EmberObject.extend();
nsB.Foo = EmberObject.extend();

assert.equal(
nsA.Foo.toString(),
'NamespaceA.Foo',
"The namespace's name is used when the namespace is not in the lookup object"
);
assert.equal(
nsB.Foo.toString(),
'CustomNamespaceB.Foo',
"The namespace's name is used when the namespace is in the lookup object"
);
try {
let nsB = (lookup.NamespaceB = Namespace.create({
name: 'CustomNamespaceB',
}));

nsA.Foo = EmberObject.extend();
nsB.Foo = EmberObject.extend();

assert.equal(
nsA.Foo.toString(),
'NamespaceA.Foo',
"The namespace's name is used when the namespace is not in the lookup object"
);
assert.equal(
nsB.Foo.toString(),
'CustomNamespaceB.Foo',
"The namespace's name is used when the namespace is in the lookup object"
);
} finally {
run(nsA, 'destroy');
}
}

['@test Calling namespace.nameClasses() eagerly names all classes'](assert) {
Expand Down Expand Up @@ -138,6 +147,8 @@ moduleFor(
UI.Nav = Namespace.create();

assert.equal(Namespace.byName('UI.Nav'), UI.Nav);

run(UI.Nav, 'destroy');
}

['@test Destroying a namespace before caching lookup removes it from the list of namespaces'](
Expand Down
11 changes: 11 additions & 0 deletions packages/ember-runtime/tests/system/object/toString_test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { run } from '@ember/runloop';
import { guidFor, setName } from 'ember-utils';
import { context } from 'ember-environment';
import EmberObject from '../../../lib/system/object';
Expand Down Expand Up @@ -35,6 +36,8 @@ moduleFor(
assert.equal(obj.toString(), '<Foo.Bar:' + guidFor(obj) + '>');

assert.equal(Foo.Bar.toString(), 'Foo.Bar');

run(Foo, 'destroy');
}

['@test toString on a class returns a useful value when nested in a namespace'](assert) {
Expand All @@ -59,12 +62,16 @@ moduleFor(

obj = Foo.Bar.create();
assert.equal(obj.toString(), '<Foo.Bar:' + guidFor(obj) + '>');

run(Foo, 'destroy');
}

['@test toString on a namespace finds the namespace in lookup'](assert) {
let Foo = (lookup.Foo = Namespace.create());

assert.equal(Foo.toString(), 'Foo');

run(Foo, 'destroy');
}

['@test toString on a namespace finds the namespace in lookup'](assert) {
Expand All @@ -77,12 +84,16 @@ moduleFor(

obj = Foo.Bar.create();
assert.equal(obj.toString(), '<Foo.Bar:' + guidFor(obj) + '>');

run(Foo, 'destroy');
}

['@test toString on a namespace falls back to modulePrefix, if defined'](assert) {
let Foo = Namespace.create({ modulePrefix: 'foo' });

assert.equal(Foo.toString(), 'foo');

run(Foo, 'destroy');
}

['@test toString includes toStringExtension if defined'](assert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function checkTemplate(templateName, assert) {
runAppend(component);

assert.equal(qunitFixture.textContent.trim(), 'Tobias takes teamocil', 'template works');
runDestroy(component);
runDestroy(owner);
}

moduleFor(
Expand All @@ -49,7 +49,7 @@ moduleFor(

teardown() {
setTemplates({});
runDestroy(component);
fixture = component = null;
}

['@test template with data-template-name should add a new template to Ember.TEMPLATES'](
Expand Down
Loading

0 comments on commit 7d494bb

Please sign in to comment.