Merge pull request #395 from matrix-org/kegan/memleaks

Fix leaking room state objects on limited sync responses

src/client.js

@@ -2733,10 +2733,10 @@ MatrixClient.prototype.getTurnServers = function() {
 
 
 /**
- * High level helper method to call initialSync, emit the resulting events,
- * and then start polling the eventStream for new events. To listen for these
+ * High level helper method to begin syncing and poll for new events. To listen for these
  * events, add a listener for {@link module:client~MatrixClient#event:"event"}
- * via {@link module:client~MatrixClient#on}.
+ * via {@link module:client~MatrixClient#on}. Alternatively, listen for specific
+ * state change events.
  * @param {Object=} opts Options to apply when syncing.
  * @param {Number=} opts.initialSyncLimit The event <code>limit=</code> to apply
  * to initial sync. Default: 8.
@@ -2753,7 +2753,7 @@ MatrixClient.prototype.getTurnServers = function() {
  * accessbile via {@link module:models/room#getPendingEvents}. Default:
  * "chronological".
  *
- * @param {Number=} opts.pollTimeout The number of milliseconds to wait on /events.
+ * @param {Number=} opts.pollTimeout The number of milliseconds to wait on /sync.
  * Default: 30000 (30 seconds).
  *
  * @param {Filter=} opts.filter The filter to apply to /sync calls. This will override
@@ -2790,6 +2790,12 @@ MatrixClient.prototype.startClient = function(opts) {
 
     opts.crypto = this._crypto;
     opts.syncAccumulator = this._syncAccumulator;
+    opts.canResetEntireTimeline = (roomId) => {
+        if (!this._canResetTimelineCallback) {
+            return false;
+        }
+        return this._canResetTimelineCallback(roomId);
+    };
     this._clientOpts = opts;
 
     this._syncApi = new SyncApi(this, opts);
@@ -2810,6 +2816,27 @@ MatrixClient.prototype.stopClient = function() {
     global.clearTimeout(this._checkTurnServersTimeoutID);
 };
 
+/*
+ * Set a function which is called when /sync returns a 'limited' response.
+ * It is called with a room ID and returns a boolean. It should return 'true' if the SDK
+ * can SAFELY remove events from this room. It may not be safe to remove events if there
+ * are other references to the timelines for this room, e.g because the client is
+ * actively viewing events in this room.
+ * Default: returns false.
+ * @param {Function} cb The callback which will be invoked.
+ */
+MatrixClient.prototype.setCanResetTimelineCallback = function(cb) {
+    this._canResetTimelineCallback = cb;
+};
+
+/**
+ * Get the callback set via `setCanResetTimelineCallback`.
+ * @return {?Function} The callback or null
+ */
+MatrixClient.prototype.getCanResetTimelineCallback = function() {
+    return this._canResetTimelineCallback;
+};
+
 function setupCallEventHandler(client) {
     const candidatesByCall = {
         // callId: [Candidate]

src/models/event-timeline-set.js

@@ -154,10 +154,11 @@ EventTimelineSet.prototype.replaceEventId = function(oldEventId, newEventId) {
  * @fires module:client~MatrixClient#event:"Room.timelineReset"
  */
 EventTimelineSet.prototype.resetLiveTimeline = function(backPaginationToken, flush) {
-    let newTimeline;
+    // if timeline support is disabled, forget about the old timelines
+    const resetAllTimelines = !this._timelineSupport || flush;
 
-    if (!this._timelineSupport || flush) {
-        // if timeline support is disabled, forget about the old timelines
+    let newTimeline;
+    if (resetAllTimelines) {
         newTimeline = new EventTimeline(this);
         this._timelines = [newTimeline];
         this._eventIdToTimeline = {};
@@ -187,7 +188,7 @@ EventTimelineSet.prototype.resetLiveTimeline = function(backPaginationToken, flu
     newTimeline.setPaginationToken(backPaginationToken, EventTimeline.BACKWARDS);
 
     this._liveTimeline = newTimeline;
-    this.emit("Room.timelineReset", this.room, this);
+    this.emit("Room.timelineReset", this.room, this, resetAllTimelines);
 };
 
 /**
@@ -655,4 +656,5 @@ module.exports = EventTimelineSet;
  * @event module:client~MatrixClient#"Room.timelineReset"
  * @param {Room} room The room whose live timeline was reset, if any
  * @param {EventTimelineSet} timelineSet timelineSet room whose live timeline was reset
+ * @param {boolean} resetAllTimelines True if all timelines were reset.
  */

src/models/event.js

@@ -49,6 +49,8 @@ module.exports.EventStatus = {
     CANCELLED: "cancelled",
 };
 
+const interns = {};
+
 /**
  * Construct a Matrix Event object
  * @constructor
@@ -75,7 +77,31 @@ module.exports.EventStatus = {
 module.exports.MatrixEvent = function MatrixEvent(
     event,
 ) {
+    // intern the values of matrix events to force share strings and reduce the
+    // amount of needless string duplication. This can save moderate amounts of
+    // memory (~10% on a 350MB heap).
+    ["state_key", "type", "sender", "room_id"].forEach((prop) => {
+        if (!event[prop]) {
+            return;
+        }
+        if (!interns[event[prop]]) {
+            interns[event[prop]] = event[prop];
+        }
+        event[prop] = interns[event[prop]];
+    });
+
+    ["membership", "avatar_url", "displayname"].forEach((prop) => {
+        if (!event.content || !event.content[prop]) {
+            return;
+        }
+        if (!interns[event.content[prop]]) {
+            interns[event.content[prop]] = event.content[prop];
+        }
+        event.content[prop] = interns[event.content[prop]];
+    });
+
     this.event = event || {};
+
     this.sender = null;
     this.target = null;
     this.status = null;

src/models/room.js

@@ -204,10 +204,12 @@ Room.prototype.getLiveTimeline = function() {
  * <p>This is used when /sync returns a 'limited' timeline.
  *
  * @param {string=} backPaginationToken   token for back-paginating the new timeline
+ * @param {boolean=} flush True to remove all events in all timelines. If false, only
+ * the live timeline is reset.
  */
-Room.prototype.resetLiveTimeline = function(backPaginationToken) {
+Room.prototype.resetLiveTimeline = function(backPaginationToken, flush) {
     for (let i = 0; i < this._timelineSets.length; i++) {
-        this._timelineSets[i].resetLiveTimeline(backPaginationToken);
+        this._timelineSets[i].resetLiveTimeline(backPaginationToken, flush);
     }
 
     this._fixUpLegacyTimelineFields();

src/sync.js

@@ -63,6 +63,11 @@ function debuglog() {
  * @param {SyncAccumulator=} opts.syncAccumulator An accumulator which will be
  * kept up-to-date. If one is supplied, the response to getJSON() will be used
  * initially.
+ * @param {Function=} opts.canResetEntireTimeline A function which is called
+ * with a room ID and returns a boolean. It should return 'true' if the SDK can
+ * SAFELY remove events from this room. It may not be safe to remove events if
+ * there are other references to the timelines for this room.
+ * Default: returns false.
  */
 function SyncApi(client, opts) {
     this.client = client;
@@ -73,6 +78,11 @@ function SyncApi(client, opts) {
     opts.resolveInvitesToProfiles = opts.resolveInvitesToProfiles || false;
     opts.pollTimeout = opts.pollTimeout || (30 * 1000);
     opts.pendingEventOrdering = opts.pendingEventOrdering || "chronological";
+    if (!opts.canResetEntireTimeline) {
+        opts.canResetEntireTimeline = function(roomId) {
+            return false;
+        };
+    }
     this.opts = opts;
     this._peekRoomId = null;
     this._currentSyncRequest = null;
@@ -848,7 +858,10 @@ SyncApi.prototype._processSyncResponse = function(syncToken, data) {
                 // timeline.
                 room.currentState.paginationToken = syncToken;
                 self._deregisterStateListeners(room);
-                room.resetLiveTimeline(joinObj.timeline.prev_batch);
+                room.resetLiveTimeline(
+                    joinObj.timeline.prev_batch,
+                    self.opts.canResetEntireTimeline(room.roomId),
+                );
 
                 // We have to assume any gap in any timeline is
                 // reason to stop incrementally tracking notifications and