From 34e455c6fcdab56ec404ed8f0b677d402850a823 Mon Sep 17 00:00:00 2001
From: Luke Barnard <luke@matrix.org>
Date: Fri, 15 Dec 2017 14:12:21 +0000
Subject: [PATCH 1/2] Fix leaking of GroupStore listeners in RoomList

---
 src/components/views/rooms/RoomList.js | 16 ++++++++++++----
 src/stores/GroupStore.js               |  6 ++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js
index d94fef7d31..8c2bdd5994 100644
--- a/src/components/views/rooms/RoomList.js
+++ b/src/components/views/rooms/RoomList.js
@@ -86,6 +86,7 @@ module.exports = React.createClass({
 
         const dmRoomMap = DMRoomMap.shared();
         this._groupStores = {};
+        this._groupStoreTokens = [];
         // A map between tags which are group IDs and the room IDs of rooms that should be kept
         // in the room list when filtering by that tag.
         this._selectedTagsRoomIdsForGroup = {
@@ -100,10 +101,12 @@ module.exports = React.createClass({
                     return;
                 }
                 this._groupStores[tag] = GroupStoreCache.getGroupStore(tag);
-                this._groupStores[tag].registerListener(() => {
-                    // This group's rooms or members may have updated, update rooms for its tag
-                    this.updateSelectedTagsRooms(dmRoomMap, [tag]);
-                });
+                this._groupStoreTokens.push(
+                    this._groupStores[tag].registerListener(() => {
+                        // This group's rooms or members may have updated, update rooms for its tag
+                        this.updateSelectedTagsRooms(dmRoomMap, [tag]);
+                    }),
+                );
             });
             // Filters themselves have changed, refresh the selected tags
             this.updateSelectedTagsRooms(dmRoomMap, FilterStore.getSelectedTags());
@@ -182,6 +185,11 @@ module.exports = React.createClass({
             this._filterStoreToken.remove();
         }
 
+        if (this._groupStoreTokens.length > 0) {
+            // NB: GroupStore is not a Flux.Store
+            this._groupStoreTokens.forEach((token) => token.unregister());
+        }
+
         // cancel any pending calls to the rate_limited_funcs
         this._delayedRefreshRoomList.cancelPendingCall();
     },
diff --git a/src/stores/GroupStore.js b/src/stores/GroupStore.js
index 8d95d1975b..483eed96a5 100644
--- a/src/stores/GroupStore.js
+++ b/src/stores/GroupStore.js
@@ -110,6 +110,12 @@ export default class GroupStore extends EventEmitter {
         this._fetchSummary();
         this._fetchRooms();
         this._fetchMembers();
+
+        return {
+            unregister: () => {
+                this.unregisterListener(fn);
+            },
+        };
     }
 
     unregisterListener(fn) {

From 45e860de7a8c91d41e9d3a81656d91c2aad03a03 Mon Sep 17 00:00:00 2001
From: lukebarnard <luke.barnard99@gmail.com>
Date: Tue, 2 Jan 2018 18:12:08 +0000
Subject: [PATCH 2/2] Document GroupStore.registerListener

---
 src/stores/GroupStore.js | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/stores/GroupStore.js b/src/stores/GroupStore.js
index 483eed96a5..9dce15fb53 100644
--- a/src/stores/GroupStore.js
+++ b/src/stores/GroupStore.js
@@ -103,6 +103,22 @@ export default class GroupStore extends EventEmitter {
         this.emit('update');
     }
 
+    /**
+     * Register a listener to recieve updates from the store. This also
+     * immediately triggers an update to send the current state of the
+     * store (which could be the initial state).
+     *
+     * XXX: This also causes a fetch of all group data, which effectively
+     * causes 4 separate HTTP requests. This is bad, we should at least
+     * deduplicate these in order to fix:
+     *  https://github.com/vector-im/riot-web/issues/5901
+     *
+     * @param {function} fn the function to call when the store updates.
+     * @return {Object} tok a registration "token" with a single
+     *                      property `unregister`, a function that can
+     *                      be called to unregister the listener such
+     *                      that it won't be called any more.
+     */
     registerListener(fn) {
         this.on('update', fn);
         // Call to set initial state (before fetching starts)
@@ -111,6 +127,8 @@ export default class GroupStore extends EventEmitter {
         this._fetchRooms();
         this._fetchMembers();
 
+        // Similar to the Store of flux/utils, we return a "token" that
+        // can be used to unregister the listener.
         return {
             unregister: () => {
                 this.unregisterListener(fn);