Skip to content

Commit 56335c5

Browse files
chrisbiancachrisbianca
committed
Fix JS reference memory leaks
1 parent da8b175 commit 56335c5

File tree

2 files changed

+91
-184
lines changed

2 files changed

+91
-184
lines changed

‎lib/modules/database/index.js

Lines changed: 76 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -22,30 +22,22 @@ export default class Database extends Base {
2222
this.log.debug('Created new Database instance', this.options);
2323

2424
this.persistenceEnabled = false;
25-
this.successListener = null;
26-
this.errorListener = null;
27-
this.refs = {};
28-
this.dbSubscriptions = {}; // { path: { modifier: { eventType: [Subscriptions] } } }
25+
this.successListener = FirestackDatabaseEvt
26+
.addListener(
27+
'database_event',
28+
event => this.handleDatabaseEvent(event));
29+
this.errorListener = FirestackDatabaseEvt
30+
.addListener(
31+
'database_error',
32+
err => this.handleDatabaseError(err));
33+
34+
this.dbSubscriptions = {};
2935
}
3036

3137
ref(...path: Array<string>) {
3238
return new Reference(this, path);
3339
}
3440

35-
storeRef(key: string, instance: Reference): Promise<Reference> {
36-
if (!this.refs[key]) {
37-
this.refs[key] = instance;
38-
}
39-
return Promise.resolve(this.refs[key]);
40-
}
41-
42-
unstoreRef(key: string): Promise<void> {
43-
if (this.refs[key]) {
44-
delete this.refs[key];
45-
}
46-
return Promise.resolve();
47-
}
48-
4941
setPersistence(enable: boolean = true) {
5042
let promise;
5143
if (this.persistenceEnabled !== enable) {
@@ -59,148 +51,94 @@ export default class Database extends Base {
5951
return promise;
6052
}
6153

62-
handleDatabaseEvent(evt: Object) {
63-
const body = evt.body || {};
64-
const path = body.path;
65-
const modifiersString = body.modifiersString || '';
66-
const modifier = modifiersString;
67-
const eventName = body.eventName;
68-
this.log.debug('handleDatabaseEvent: ', path, modifiersString, eventName, body.snapshot && body.snapshot.key);
69-
70-
// subscriptionsMap === { path: { modifier: { eventType: [Subscriptions] } } }
71-
const modifierMap = this.dbSubscriptions[path];
72-
if (modifierMap) {
73-
const eventTypeMap = modifierMap[modifier];
74-
if (eventTypeMap) {
75-
const callbacks = eventTypeMap[eventName] || [];
76-
this.log.debug(' -- about to fire its ' + callbacks.length + ' callbacks');
77-
callbacks.forEach(cb => {
78-
if (cb && typeof(cb) === 'function') {
79-
const snap = new Snapshot(this, body.snapshot);
80-
cb(snap, body);
81-
}
82-
});
83-
}
54+
handleDatabaseEvent(event: Object) {
55+
const body = event.body || {};
56+
const { path, modifiersString, eventName, snapshot } = body;
57+
const dbHandle = this._dbHandle(path, modifiersString);
58+
this.log.debug('handleDatabaseEvent: ', dbHandle, eventName, snapshot && snapshot.key);
59+
60+
if (this.dbSubscriptions[dbHandle] && this.dbSubscriptions[dbHandle][eventName]) {
61+
this.dbSubscriptions[dbHandle][eventName].forEach(cb => {
62+
if (cb && typeof(cb) === 'function') {
63+
const snap = new Snapshot(this, snapshot);
64+
cb(snap, body);
65+
}
66+
})
67+
} else {
68+
FirestackDatabase.off(path, modifiersString, eventName, () => {
69+
this.log.debug('handleDatabaseEvent: No JS listener registered, removed native listener', dbHandle, eventName);
70+
});
8471
}
8572
}
8673

87-
handleDatabaseError(evt: Object) {
88-
this.log.debug('handleDatabaseError ->', evt);
74+
handleDatabaseError(err: Object) {
75+
this.log.debug('handleDatabaseError ->', err);
8976
}
9077

91-
on(referenceKey: string, path: string, modifiersString: string, modifiers: Array<string>, evt: string, cb: () => void) {
92-
this.log.debug('adding on listener', referenceKey, path, modifiers, evt);
93-
const key = this._pathKey(path);
94-
95-
if (!this.dbSubscriptions[key]) {
96-
this.dbSubscriptions[key] = {};
97-
}
98-
99-
if (!this.dbSubscriptions[key][modifiersString]) {
100-
this.dbSubscriptions[key][modifiersString] = {};
101-
}
102-
103-
if (!this.dbSubscriptions[key][modifiersString][evt]) {
104-
this.dbSubscriptions[key][modifiersString][evt] = [];
105-
}
106-
107-
this.dbSubscriptions[key][modifiersString][evt].push(cb);
108-
109-
if (!this.successListener) {
110-
this.successListener = FirestackDatabaseEvt
111-
.addListener(
112-
'database_event',
113-
this.handleDatabaseEvent.bind(this));
114-
}
115-
116-
if (!this.errorListener) {
117-
this.errorListener = FirestackDatabaseEvt
118-
.addListener(
119-
'database_error',
120-
this.handleDatabaseError.bind(this));
78+
on(path: string, modifiersString: string, modifiers: Array<string>, eventName: string, cb: () => void) {
79+
const dbHandle = this._dbHandle(path, modifiersString);
80+
this.log.debug('adding on listener', dbHandle);
81+
82+
if (this.dbSubscriptions[dbHandle]) {
83+
if (this.dbSubscriptions[dbHandle][eventName]) {
84+
this.dbSubscriptions[dbHandle][eventName].push(cb);
85+
} else {
86+
this.dbSubscriptions[dbHandle][eventName] = [cb];
87+
}
88+
} else {
89+
this.dbSubscriptions[dbHandle] = {
90+
[eventName]: [cb]
91+
}
12192
}
12293

123-
return promisify('on', FirestackDatabase)(path, modifiersString, modifiers, evt).then(() => {
124-
return [this.successListener, this.errorListener];
125-
});
94+
return promisify('on', FirestackDatabase)(path, modifiersString, modifiers, eventName);
12695
}
12796

128-
off(referenceKey: string, path: string, modifiersString: string, modifiers: Array<string>, eventName: string, origCB?: () => void) {
129-
const pathKey = this._pathKey(path);
130-
this.log.debug('off() : ', referenceKey, pathKey, modifiersString, eventName);
131-
// Remove subscription
132-
if (this.dbSubscriptions[pathKey]) {
97+
off(path: string, modifiersString: string, eventName?: string, origCB?: () => void) {
98+
const dbHandle = this._dbHandle(path, modifiersString);
99+
this.log.debug('off() : ', dbHandle, eventName);
133100

134-
if (!eventName || eventName === '') {
135-
// remove all listeners for this pathKey
136-
this.dbSubscriptions[pathKey] = {};
137-
}
101+
if (!this.dbSubscriptions[dbHandle]
102+
|| (eventName && !this.dbSubscriptions[dbHandle][eventName])) {
103+
this.log.warn('off() called, but not currently listening at that location (bad path)', dbHandle, eventName);
104+
return Promise.resolve();
105+
}
138106

139-
// TODO clean me - no need for this many conditionals
140-
if (this.dbSubscriptions[pathKey][modifiersString]) {
141-
if (this.dbSubscriptions[pathKey][modifiersString][eventName]) {
142-
if (origCB) {
143-
// remove only the given callback
144-
this.dbSubscriptions[pathKey][modifiersString][eventName].splice(this.dbSubscriptions[pathKey][modifiersString][eventName].indexOf(origCB), 1);
145-
} else {
146-
// remove all callbacks for this path:modifier:eventType
147-
delete this.dbSubscriptions[pathKey][modifiersString][eventName];
148-
}
149-
} else {
150-
this.log.warn('off() called, but not currently listening at that location (bad eventName)', pathKey, modifiersString, eventName);
151-
}
107+
if (eventName && origCB) {
108+
const i = this.dbSubscriptions[dbHandle][eventName].indexOf(origCB);
109+
if (i === -1) {
110+
this.log.warn('off() called, but the callback specifed is not listening at that location (bad path)', dbHandle, eventName);
111+
return Promise.resolve();
152112
} else {
153-
this.log.warn('off() called, but not currently listening at that location (bad modifier)', pathKey, modifiersString, eventName);
154-
}
155-
156-
if (Object.keys(this.dbSubscriptions[pathKey]).length <= 0) {
157-
// there are no more subscriptions so we can unwatch
158-
delete this.dbSubscriptions[pathKey];
159-
}
160-
if (Object.keys(this.dbSubscriptions).length === 0) {
161-
if (this.successListener) {
162-
this.successListener.remove();
163-
this.successListener = null;
164-
}
165-
if (this.errorListener) {
166-
this.errorListener.remove();
167-
this.errorListener = null;
113+
this.dbSubscriptions[dbHandle][eventName] = this.dbSubscriptions[dbHandle][eventName].splice(i, 1);
114+
if (this.dbSubscriptions[dbHandle][eventName].length > 0) {
115+
return Promise.resolve();
168116
}
169117
}
118+
} else if (eventName) {
119+
this.dbSubscriptions[dbHandle][eventName] = [];
170120
} else {
171-
this.log.warn('off() called, but not currently listening at that location (bad path)', pathKey, modifiersString, eventName);
172-
}
173-
174-
const subscriptions = [this.successListener, this.errorListener];
175-
const modifierMap = this.dbSubscriptions[path];
176-
177-
if (modifierMap && modifierMap[modifiersString] && modifierMap[modifiersString][eventName] && modifierMap[modifiersString][eventName].length > 0) {
178-
return Promise.resolve(subscriptions);
121+
this.dbSubscriptions[dbHandle] = {}
179122
}
180-
181-
return promisify('off', FirestackDatabase)(path, modifiersString, eventName).then(() => {
182-
// subscriptions.forEach(sub => sub.remove());
183-
// delete this.listeners[eventName];
184-
return subscriptions;
185-
});
123+
return promisify('off', FirestackDatabase)(path, modifiersString, eventName);
186124
}
187125

188126
cleanup() {
189-
let promises = Object.keys(this.refs)
190-
.map(key => this.refs[key])
191-
.map(ref => ref.cleanup());
127+
let promises = [];
128+
Object.keys(this.dbSubscriptions).forEach(dbHandle => {
129+
Object.keys(this.dbSubscriptions[dbHandle]).forEach(eventName => {
130+
let separator = dbHandle.indexOf('|');
131+
let path = dbHandle.substring(0, separator);
132+
let modifiersString = dbHandle.substring(separator + 1);
133+
134+
promises.push(this.off(path, modifiersString, eventName))
135+
})
136+
})
192137
return Promise.all(promises);
193138
}
194139

195-
release(...path: Array<string>) {
196-
const key = this._pathKey(...path);
197-
if (this.refs[key]) {
198-
delete this.refs[key];
199-
}
200-
}
201-
202-
_pathKey(...path: Array<string>): string {
203-
return path.join('-');
140+
_dbHandle(path: string = '', modifiersString: string = '') {
141+
return path + '|' + modifiersString;
204142
}
205143

206144
goOnline() {

‎lib/modules/database/reference.js

Lines changed: 15 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import { promisify, isFunction, isObject, tryJSONParse, tryJSONStringify, genera
1212
const FirestackDatabase = NativeModules.FirestackDatabase;
1313

1414
// https://firebase.google.com/docs/reference/js/firebase.database.Reference
15-
let uid = 0;
1615

1716
/**
1817
* @class Reference
@@ -21,16 +20,13 @@ export default class Reference extends ReferenceBase {
2120

2221
db: FirestackDatabase;
2322
query: Query;
24-
uid: number;
2523

2624
constructor(db: FirestackDatabase, path: Array<string>, existingModifiers?: Array<string>) {
2725
super(db.firestack, path);
2826

2927
this.db = db;
30-
this.uid = uid += 1;
31-
this.listeners = {};
3228
this.query = new Query(this, path, existingModifiers);
33-
this.log.debug('Created new Reference', this.dbPath(), this.uid);
29+
this.log.debug('Created new Reference', this.db._dbHandle(path, existingModifiers));
3430
}
3531

3632
child(...paths: Array<string>) {
@@ -42,15 +38,6 @@ export default class Reference extends ReferenceBase {
4238
return promisify('keepSynced', FirestackDatabase)(path, bool);
4339
}
4440

45-
// Get the value of a ref either with a key
46-
// todo - where is this on the FB JS api - seems just like another random function
47-
get() {
48-
const path = this.dbPath();
49-
const modifiers = this.query.getModifiers();
50-
const modifiersString = this.query.getModifiersString();
51-
return promisify('onOnce', FirestackDatabase)(path, modifiersString, modifiers, 'value');
52-
}
53-
5441
set(value: any) {
5542
const path = this.dbPath();
5643
const _value = this._serializeAnyType(value);
@@ -93,49 +80,31 @@ export default class Reference extends ReferenceBase {
9380
});
9481
}
9582

96-
on(evt?: string, cb: () => any) {
83+
on(eventName: string, cb: () => any) {
9784
const path = this.dbPath();
9885
const modifiers = this.query.getModifiers();
9986
const modifiersString = this.query.getModifiersString();
100-
this.log.debug('adding reference.on', path, modifiersString, evt);
101-
return this.db.storeRef(this.uid, this).then(() => {
102-
return this.db.on(this.uid, path, modifiersString, modifiers, evt, cb).then((subscriptions) => {
103-
this.listeners[evt] = subscriptions;
104-
});
105-
});
87+
this.log.debug('adding reference.on', path, modifiersString, eventName);
88+
return this.db.on(path, modifiersString, modifiers, eventName, cb);
10689
}
10790

108-
once(evt?: string = 'once', cb: (snapshot: Object) => void) {
91+
once(eventName: string = 'once', cb: (snapshot: Object) => void) {
10992
const path = this.dbPath();
11093
const modifiers = this.query.getModifiers();
11194
const modifiersString = this.query.getModifiersString();
112-
return this.db.storeRef(this.uid, this).then(() => {
113-
// todo use event emitter - not callbacks
114-
return promisify('onOnce', FirestackDatabase)(path, modifiersString, modifiers, evt)
115-
.then(({ snapshot }) => new Snapshot(this, snapshot))
116-
.then((snapshot) => {
117-
if (isFunction(cb)) cb(snapshot);
118-
return snapshot;
119-
});
120-
});
121-
}
122-
123-
off(evt: string = '', origCB?: () => any) {
124-
const path = this.dbPath();
125-
const modifiers = this.query.getModifiers();
126-
const modifiersString = this.query.getModifiersString();
127-
this.log.debug('ref.off(): ', path, modifiers, evt);
128-
return this.db.unstoreRef(this.uid).then(() => {
129-
return this.db.off(this.uid, path, modifiersString, modifiers, evt, origCB).then(() => {
130-
// todo urm - whats this?
131-
// delete this.listeners[eventName];
132-
// this.listeners[evt] = subscriptions;
95+
return promisify('onOnce', FirestackDatabase)(path, modifiersString, modifiers, eventName)
96+
.then(({ snapshot }) => new Snapshot(this, snapshot))
97+
.then((snapshot) => {
98+
if (isFunction(cb)) cb(snapshot);
99+
return snapshot;
133100
});
134-
});
135101
}
136102

137-
cleanup() {
138-
return Promise.all(Object.keys(this.listeners).map(key => this.off(key)));
103+
off(eventName?: string = '', origCB?: () => any) {
104+
const path = this.dbPath();
105+
const modifiersString = this.query.getModifiersString();
106+
this.log.debug('ref.off(): ', path, modifiersString, eventName);
107+
return this.db.off(path, modifiersString, eventName, origCB);
139108
}
140109

141110
/**

0 commit comments

Comments
 (0)