From ae8fc64394e00a53e6bbd5b81032b978a96776c5 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Fri, 8 Sep 2017 14:20:34 +0100
Subject: [PATCH] Do /keys/changes before second /sync

This will avoid races between /keys/changes and /syncs.
---
 src/crypto/index.js | 55 ++++++++++++++++-----------------------------
 src/sync.js         |  8 ++++++-
 2 files changed, 26 insertions(+), 37 deletions(-)

diff --git a/src/crypto/index.js b/src/crypto/index.js
index f493c9f44..026c0cc9a 100644
--- a/src/crypto/index.js
+++ b/src/crypto/index.js
@@ -69,7 +69,6 @@ function Crypto(baseApis, sessionStore, userId, deviceId,
 
     this._olmDevice = new OlmDevice(sessionStore);
     this._deviceList = new DeviceList(baseApis, sessionStore, this._olmDevice);
-    this._initialDeviceListInvalidationPending = false;
 
     // the last time we did a check for the number of one-time-keys on the
     // server.
@@ -150,15 +149,6 @@ Crypto.prototype.init = async function() {
  */
 Crypto.prototype.registerEventHandlers = function(eventEmitter) {
     const crypto = this;
-    eventEmitter.on("sync", function(syncState, oldState, data) {
-        try {
-            if (syncState === "SYNCING") {
-                crypto._onSyncCompleted(data);
-            }
-        } catch (e) {
-            console.error("Error handling sync", e);
-        }
-    });
 
     eventEmitter.on("RoomMember.membership", function(event, member, oldMembership) {
         try {
@@ -248,7 +238,7 @@ Crypto.prototype.uploadDeviceKeys = function() {
 
 /**
  * Stores the current one_time_key count which will be handled later (in a call of
- * _onSyncCompleted). The count is e.g. coming from a /sync response.
+ * onSyncCompleted). The count is e.g. coming from a /sync response.
  *
  * @param {Number} currentCount The current count of one_time_keys to be stored
  */
@@ -837,7 +827,7 @@ Crypto.prototype.onCryptoEvent = async function(event) {
 
     try {
         // inhibit the device list refresh for now - it will happen once we've
-        // finished processing the sync, in _onSyncCompleted.
+        // finished processing the sync, in onSyncCompleted.
         await this.setRoomEncryption(roomId, content, true);
     } catch (e) {
         console.error("Error configuring encryption in room " + roomId +
@@ -853,7 +843,7 @@ Crypto.prototype.onCryptoEvent = async function(event) {
  *
  * @param {Object} syncData  the data from the 'MatrixClient.sync' event
  */
-Crypto.prototype._onSyncCompleted = function(syncData) {
+Crypto.prototype.onSyncCompleted = async function(syncData) {
     const nextSyncToken = syncData.nextSyncToken;
 
     if (!syncData.oldSyncToken) {
@@ -863,18 +853,15 @@ Crypto.prototype._onSyncCompleted = function(syncData) {
         // invalidate devices which have changed since then.
         const oldSyncToken = this._sessionStore.getEndToEndDeviceSyncToken();
         if (oldSyncToken !== null) {
-            this._initialDeviceListInvalidationPending = true;
-            this._invalidateDeviceListsSince(
-                oldSyncToken, nextSyncToken,
-            ).catch((e) => {
+            try {
+                await this._invalidateDeviceListsSince(
+                    oldSyncToken, nextSyncToken,
+                );
+            } catch (e) {
                 // if that failed, we fall back to invalidating everyone.
                 console.warn("Error fetching changed device list", e);
                 this._deviceList.invalidateAllDeviceLists();
-            }).done(() => {
-                this._initialDeviceListInvalidationPending = false;
-                this._deviceList.lastKnownSyncToken = nextSyncToken;
-                this._deviceList.refreshOutdatedDeviceLists();
-            });
+            }
         } else {
             // otherwise, we have to invalidate all devices for all users we
             // are tracking.
@@ -884,14 +871,12 @@ Crypto.prototype._onSyncCompleted = function(syncData) {
         }
     }
 
-    if (!this._initialDeviceListInvalidationPending) {
-        // we can now store our sync token so that we can get an update on
-        // restart rather than having to invalidate everyone.
-        //
-        // (we don't really need to do this on every sync - we could just
-        // do it periodically)
-        this._sessionStore.storeEndToEndDeviceSyncToken(nextSyncToken);
-    }
+    // we can now store our sync token so that we can get an update on
+    // restart rather than having to invalidate everyone.
+    //
+    // (we don't really need to do this on every sync - we could just
+    // do it periodically)
+    this._sessionStore.storeEndToEndDeviceSyncToken(nextSyncToken);
 
     // catch up on any new devices we got told about during the sync.
     this._deviceList.lastKnownSyncToken = nextSyncToken;
@@ -925,13 +910,11 @@ Crypto.prototype._invalidateDeviceListsSince = function(
     ).then((r) => {
         console.log("got key changes since", oldSyncToken, ":", r.changed);
 
-        if (!r.changed || !Array.isArray(r.changed)) {
-            return;
+        if (r.changed && Array.isArray(r.changed)) {
+            r.changed.forEach((u) => {
+                this._deviceList.invalidateUserDeviceList(u);
+            });
         }
-
-        r.changed.forEach((u) => {
-            this._deviceList.invalidateUserDeviceList(u);
-        });
     });
 };
 
diff --git a/src/sync.js b/src/sync.js
index 8909b27b3..cc2ce5bac 100644
--- a/src/sync.js
+++ b/src/sync.js
@@ -634,8 +634,14 @@ SyncApi.prototype._sync = async function(syncOptions) {
         syncOptions.hasSyncedBefore = true;
     }
 
-    // keep emitting SYNCING -> SYNCING for clients who want to do bulk updates
     if (!isCachedResponse) {
+        // tell the crypto module to do its processing. It may block (to do a
+        // /keys/changes request).
+        if (this.opts.crypto) {
+            await this.opts.crypto.onSyncCompleted(syncEventData);
+        }
+
+        // keep emitting SYNCING -> SYNCING for clients who want to do bulk updates
         this._updateSyncState("SYNCING", syncEventData);
 
         // tell databases that everything is now in a consistent state and can be