Merge pull request #1664 from matrix-org/luke/fix-room-list-group-store-leak

Fix leaking of GroupStore listeners in RoomList
This commit is contained in:
Luke Barnard 2018-01-02 18:58:22 +00:00 committed by GitHub
commit 2338fba2ee
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 36 additions and 5 deletions

View file

@ -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._visibleRoomsForGroup = {
@ -100,11 +101,12 @@ module.exports = React.createClass({
return;
}
this._groupStores[tag] = GroupStoreCache.getGroupStore(tag);
this._groupStoreTokens.push(
this._groupStores[tag].registerListener(() => {
// This group's rooms or members may have updated, update rooms for its tag
this.updateVisibleRoomsForTag(dmRoomMap, tag);
this.updateVisibleRooms();
});
this.updateSelectedTagsRooms(dmRoomMap, [tag]);
}),
);
});
// Filters themselves have changed, refresh the selected tags
this.updateVisibleRooms();
@ -183,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();
},

View file

@ -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)
@ -110,6 +126,14 @@ export default class GroupStore extends EventEmitter {
this._fetchSummary();
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);
},
};
}
unregisterListener(fn) {