From 7df2997ee7220b4e541c9e9708f10d867609d910 Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Tue, 3 Nov 2015 02:41:52 -0800 Subject: [PATCH 1/3] Use a Set to store listener callbacks in JS --- lib/realm.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/lib/realm.js b/lib/realm.js index 12bef51b..4c920f88 100644 --- a/lib/realm.js +++ b/lib/realm.js @@ -28,7 +28,7 @@ class Realm { for (let i = 0, len = schema ? schema.length : 0; i < len; i++) { let item = schema[i]; let proto = item.prototype; - + if (proto && proto.schema) { schema.splice(i, 1, proto.schema); constructors[proto.schema.name] = item; @@ -42,7 +42,7 @@ class Realm { this[keys.id] = realmId; this[keys.realm] = realmId; this[keys.type] = objectTypes.REALM; - this[listenersKey] = []; + this[listenersKey] = new Set(); [ 'path', @@ -59,28 +59,24 @@ class Realm { if (name != 'change') { throw new Error("Only 'change' notification is supported."); } - this[listenersKey].push(callback); + this[listenersKey].add(callback); } removeListener(name, callback) { if (typeof callback != 'function') { - throw new Error('Realm.addListener must be passed a function!'); + throw new Error('Realm.removeListener must be passed a function!'); } if (name != 'change') { throw new Error("Only 'change' notification is supported."); } - - let index = 0; - while ((index = this[listenersKey].indexOf(callback, index)) != -1) { - this[listenersKey].splice(index, 1); - } + this[listenersKey].delete(callback); } removeAllListeners(name) { - if (name != undefined && name != 'change') { + if (name != undefined && name != 'change') { throw new Error("Only 'change' notification is supported."); } - this[listenersKey] = []; + this[listenersKey].clear(); } write(callback) { From 83869e2193609114bda68793d58261931763a201 Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Tue, 3 Nov 2015 02:43:29 -0800 Subject: [PATCH 2/3] Prevent memory leaks when adding listeners Check if the callback has already been added to the set before protecting it. Vice versa for unprotecting it when removing the listener. --- src/RJSRealm.mm | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/RJSRealm.mm b/src/RJSRealm.mm index 986241a7..82f3edaf 100644 --- a/src/RJSRealm.mm +++ b/src/RJSRealm.mm @@ -49,12 +49,16 @@ public: } void add_notification(JSObjectRef notification) { - JSValueProtect(m_context, notification); - m_notifications.insert(notification); + if (!m_notifications.count(notification)) { + JSValueProtect(m_context, notification); + m_notifications.insert(notification); + } } void remove_notification(JSObjectRef notification) { - JSValueUnprotect(m_context, notification); - m_notifications.erase(notification); + if (m_notifications.count(notification)) { + JSValueUnprotect(m_context, notification); + m_notifications.erase(notification); + } } void remove_all_notifications() { for (auto notification : m_notifications) { From 2f4bcee5cea9c753226fe982275f0661a2e0d38a Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Tue, 3 Nov 2015 14:45:06 -0800 Subject: [PATCH 3/3] Update test to ensure unique listener callback behavior --- tests/RealmTests.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/RealmTests.js b/tests/RealmTests.js index e288dd7a..dc12c852 100644 --- a/tests/RealmTests.js +++ b/tests/RealmTests.js @@ -289,8 +289,11 @@ module.exports = BaseTest.extend({ var secondNotificationCount = 0; function secondNotification(realm, name) { secondNotificationCount++; - }; - realm.addListener('change', secondNotification) + } + + // The listener should only be added once. + realm.addListener('change', secondNotification); + realm.addListener('change', secondNotification); realm.write(function() {}); TestCase.assertEqual(notificationCount, 2);