Skip to content

Commit 072a146

Browse files
krisseldenkategengler
authored andcommitted
[BUGFIX release] Fix observer leaks
(cherry picked from commit d4d6abd)
1 parent 5453761 commit 072a146

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+610
-298
lines changed
Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1 @@
1-
export {
2-
counters,
3-
deleteMeta,
4-
Meta,
5-
meta,
6-
MetaCounters,
7-
peekMeta,
8-
setMeta,
9-
UNDEFINED,
10-
} from './lib/meta';
1+
export { counters, Meta, meta, MetaCounters, peekMeta, setMeta, UNDEFINED } from './lib/meta';

packages/@ember/-internals/meta/lib/meta.ts

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ export class Meta {
156156
}
157157

158158
destroy() {
159+
if (DEBUG) {
160+
counters!.deleteCalls++;
161+
}
162+
159163
if (this.isMetaDestroyed()) {
160164
return;
161165
}
@@ -647,34 +651,6 @@ export function peekMeta(obj: object): Meta | null {
647651
return null;
648652
}
649653

650-
/**
651-
Tears down the meta on an object so that it can be garbage collected.
652-
Multiple calls will have no effect.
653-
654-
@method deleteMeta
655-
@for Ember
656-
@param {Object} obj the object to destroy
657-
@return {void}
658-
@private
659-
*/
660-
export function deleteMeta(obj: object) {
661-
assert('Cannot call `deleteMeta` on null', obj !== null);
662-
assert('Cannot call `deleteMeta` on undefined', obj !== undefined);
663-
assert(
664-
`Cannot call \`deleteMeta\` on ${typeof obj}`,
665-
typeof obj === 'object' || typeof obj === 'function'
666-
);
667-
668-
if (DEBUG) {
669-
counters!.deleteCalls++;
670-
}
671-
672-
let meta = peekMeta(obj);
673-
if (meta !== null) {
674-
meta.destroy();
675-
}
676-
}
677-
678654
/**
679655
Retrieves the meta hash for an object. If `writable` is true ensures the
680656
hash is writable for this object as well.

packages/@ember/-internals/metal/index.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,15 @@ export { default as getProperties } from './lib/get_properties';
5050
export { default as setProperties } from './lib/set_properties';
5151
export { default as expandProperties } from './lib/expand_properties';
5252

53-
export { addObserver, activateObserver, removeObserver, flushAsyncObservers } from './lib/observer';
53+
export { destroy } from './lib/destroy';
54+
export {
55+
ASYNC_OBSERVERS,
56+
SYNC_OBSERVERS,
57+
addObserver,
58+
activateObserver,
59+
removeObserver,
60+
flushAsyncObservers,
61+
} from './lib/observer';
5462
export { Mixin, aliasMethod, mixin, observer, applyMixin } from './lib/mixin';
5563
export { default as inject, DEBUG_INJECTION_FUNCTIONS } from './lib/injected_property';
5664
export { tagForProperty, tagForObject, markObjectAsDirty, CUSTOM_TAG_FOR } from './lib/tags';
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { Meta, peekMeta } from '@ember/-internals/meta/lib/meta';
2+
import { assert } from '@ember/debug';
3+
import { schedule } from '@ember/runloop';
4+
import { destroyObservers } from './observer';
5+
6+
/**
7+
Enqueues finalization on an object so that it can be garbage collected.
8+
Multiple calls will have no effect.
9+
10+
@method destroy
11+
@for Ember
12+
@param {Object} obj the object to destroy
13+
@return {boolean} true if the object went from not destroying to destroying.
14+
@private
15+
*/
16+
export function destroy(obj: object): boolean {
17+
assert('Cannot call `destroy` on null', obj !== null);
18+
assert('Cannot call `destroy` on undefined', obj !== undefined);
19+
assert(
20+
`Cannot call \`destroy\` on ${typeof obj}`,
21+
typeof obj === 'object' || typeof obj === 'function'
22+
);
23+
24+
const m = peekMeta(obj);
25+
if (m === null || m.isSourceDestroying()) {
26+
return false;
27+
}
28+
m.setSourceDestroying();
29+
destroyObservers(obj);
30+
schedule('destroy', m, finalize);
31+
return true;
32+
}
33+
34+
function finalize(this: Meta) {
35+
this.setSourceDestroyed();
36+
this.destroy();
37+
}

packages/@ember/-internals/metal/lib/events.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,7 @@ export function sendEvent(
124124
) {
125125
if (actions === undefined) {
126126
let meta = _meta === undefined ? peekMeta(obj) : _meta;
127-
actions =
128-
typeof meta === 'object' && meta !== null ? meta.matchingListeners(eventName) : undefined;
127+
actions = meta !== null ? meta.matchingListeners(eventName) : undefined;
129128
}
130129

131130
if (actions === undefined || actions.length === 0) {

packages/@ember/-internals/metal/lib/observer.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ interface ActiveObserver {
1515
}
1616

1717
const SYNC_DEFAULT = !ENV._DEFAULT_ASYNC_OBSERVERS;
18-
const SYNC_OBSERVERS: Map<object, Map<string, ActiveObserver>> = new Map();
19-
const ASYNC_OBSERVERS: Map<object, Map<string, ActiveObserver>> = new Map();
18+
export const SYNC_OBSERVERS: Map<object, Map<string, ActiveObserver>> = new Map();
19+
export const ASYNC_OBSERVERS: Map<object, Map<string, ActiveObserver>> = new Map();
2020

2121
/**
2222
@module @ember/object
@@ -153,15 +153,16 @@ export function revalidateObservers(target: object) {
153153
let lastKnownRevision = 0;
154154

155155
export function flushAsyncObservers(shouldSchedule = true) {
156-
if (lastKnownRevision === value(CURRENT_TAG)) {
156+
let currentRevision = value(CURRENT_TAG);
157+
if (lastKnownRevision === currentRevision) {
157158
return;
158159
}
159-
160-
lastKnownRevision = value(CURRENT_TAG);
160+
lastKnownRevision = currentRevision;
161161

162162
ASYNC_OBSERVERS.forEach((activeObservers, target) => {
163163
let meta = peekMeta(target);
164164

165+
// if observer target is destroyed remove observers
165166
if (meta && (meta.isSourceDestroying() || meta.isMetaDestroyed())) {
166167
ASYNC_OBSERVERS.delete(target);
167168
return;
@@ -171,7 +172,7 @@ export function flushAsyncObservers(shouldSchedule = true) {
171172
if (!validate(observer.tag, observer.lastRevision)) {
172173
let sendObserver = () => {
173174
try {
174-
sendEvent(target, eventName, [target, observer.path]);
175+
sendEvent(target, eventName, [target, observer.path], undefined, meta);
175176
} finally {
176177
observer.tag = combine(getChainTagsForKey(target, observer.path));
177178
observer.lastRevision = value(observer.tag);
@@ -205,7 +206,7 @@ export function flushSyncObservers() {
205206
if (!observer.suspended && !validate(observer.tag, observer.lastRevision)) {
206207
try {
207208
observer.suspended = true;
208-
sendEvent(target, eventName, [target, observer.path]);
209+
sendEvent(target, eventName, [target, observer.path], undefined, meta);
209210
} finally {
210211
observer.tag = combine(getChainTagsForKey(target, observer.path));
211212
observer.lastRevision = value(observer.tag);
@@ -229,3 +230,8 @@ export function setObserverSuspended(target: object, property: string, suspended
229230
observer.suspended = suspended;
230231
}
231232
}
233+
234+
export function destroyObservers(target: object) {
235+
if (SYNC_OBSERVERS.size > 0) SYNC_OBSERVERS.delete(target);
236+
if (ASYNC_OBSERVERS.size > 0) ASYNC_OBSERVERS.delete(target);
237+
}

packages/@ember/-internals/metal/tests/accessors/get_test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { ENV } from '@ember/-internals/environment';
22
import { Object as EmberObject } from '@ember/-internals/runtime';
3+
import { destroy } from '@ember/-internals/metal';
34
import { get, set, getWithDefault, Mixin, observer, computed } from '../..';
45
import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
56
import { run } from '@ember/runloop';
@@ -196,6 +197,8 @@ moduleFor(
196197
'foo',
197198
'should return the set value, not false'
198199
);
200+
201+
run(() => destroy(baseObject));
199202
}
200203
}
201204
);
@@ -332,6 +335,8 @@ moduleFor(
332335
'foo',
333336
'should return the set value, not false'
334337
);
338+
339+
run(() => destroy(baseObject));
335340
}
336341

337342
['@test should respect prototypical inheritance when subclasses override CPs'](assert) {

packages/@ember/-internals/metal/tests/alias_test.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {
22
alias,
33
computed,
44
defineProperty,
5+
destroy,
56
get,
67
set,
78
addObserver,
@@ -27,7 +28,11 @@ moduleFor(
2728
}
2829

2930
afterEach() {
30-
obj = null;
31+
if (obj !== undefined) {
32+
destroy(obj);
33+
obj = undefined;
34+
return runLoopSettled();
35+
}
3136
}
3237

3338
['@test should proxy get to alt key'](assert) {

0 commit comments

Comments
 (0)