From 234c26033968a319fbd086d54a79cc72b82e675b Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Mon, 30 Mar 2020 20:46:52 +0200
Subject: [PATCH] dont modify fragments in comparer until txn succeeds

---
 doc/impl-thoughts/RECONNECTING.md                 |  1 +
 src/matrix/room/room.js                           | 11 +++++++----
 src/matrix/room/timeline/persistence/GapWriter.js | 14 +++++++++-----
 3 files changed, 17 insertions(+), 9 deletions(-)
 create mode 100644 doc/impl-thoughts/RECONNECTING.md

diff --git a/doc/impl-thoughts/RECONNECTING.md b/doc/impl-thoughts/RECONNECTING.md
new file mode 100644
index 00000000..fb9ff506
--- /dev/null
+++ b/doc/impl-thoughts/RECONNECTING.md
@@ -0,0 +1 @@
+# Reconnecting
\ No newline at end of file
diff --git a/src/matrix/room/room.js b/src/matrix/room/room.js
index 7de0d92b..605535a6 100644
--- a/src/matrix/room/room.js
+++ b/src/matrix/room/room.js
@@ -75,7 +75,7 @@ export default class Room extends EventEmitter {
             this._storage.storeNames.timelineFragments,
         ]);
         let removedPendingEvents;
-        let newEntries;
+        let gapResult;
         try {
             // detect remote echos of pending messages in the gap
             removedPendingEvents = this._sendQueue.removeRemoteEchos(response.chunk, txn);
@@ -85,18 +85,21 @@ export default class Room extends EventEmitter {
                 storage: this._storage,
                 fragmentIdComparer: this._fragmentIdComparer
             });
-            newEntries = await gapWriter.writeFragmentFill(fragmentEntry, response, txn);
+            gapResult = await gapWriter.writeFragmentFill(fragmentEntry, response, txn);
         } catch (err) {
             txn.abort();
             throw err;
         }
         await txn.complete();
-        // once txn is committed, emit events
+        // once txn is committed, update in-memory state & emit events
+        for (const fragment of gapResult.fragments) {
+            this._fragmentIdComparer.add(fragment);
+        }
         if (removedPendingEvents) {
             this._sendQueue.emitRemovals(removedPendingEvents);
         }
         if (this._timeline) {
-            this._timeline.addGapEntries(newEntries);
+            this._timeline.addGapEntries(gapResult.entries);
         }
     }
 
diff --git a/src/matrix/room/timeline/persistence/GapWriter.js b/src/matrix/room/timeline/persistence/GapWriter.js
index 66b1fc3d..36080270 100644
--- a/src/matrix/room/timeline/persistence/GapWriter.js
+++ b/src/matrix/room/timeline/persistence/GapWriter.js
@@ -99,6 +99,7 @@ export default class GapWriter {
 
     async _updateFragments(fragmentEntry, neighbourFragmentEntry, end, entries, txn) {
         const {direction} = fragmentEntry;
+        const changedFragments = [];
         directionalAppend(entries, fragmentEntry, direction);
         // set `end` as token, and if we found an event in the step before, link up the fragments in the fragment entry
         if (neighbourFragmentEntry) {
@@ -126,13 +127,16 @@ export default class GapWriter {
             txn.timelineFragments.update(neighbourFragmentEntry.fragment);
             directionalAppend(entries, neighbourFragmentEntry, direction);
 
-            // update fragmentIdComparer here after linking up fragments
-            this._fragmentIdComparer.add(fragmentEntry.fragment);
-            this._fragmentIdComparer.add(neighbourFragmentEntry.fragment);
+            // fragments that need to be changed in the fragmentIdComparer here
+            // after txn succeeds
+            changedFragments.push(fragmentEntry.fragment);
+            changedFragments.push(neighbourFragmentEntry.fragment);
         } else {
             fragmentEntry.token = end;
         }
         txn.timelineFragments.update(fragmentEntry.fragment);
+
+        return changedFragments;
     }
 
     async writeFragmentFill(fragmentEntry, response, txn) {
@@ -168,9 +172,9 @@ export default class GapWriter {
 
         // create entries for all events in chunk, add them to entries
         entries = this._storeEvents(nonOverlappingEvents, lastKey, direction, txn);
-        await this._updateFragments(fragmentEntry, neighbourFragmentEntry, end, entries, txn);
+        const fragments = await this._updateFragments(fragmentEntry, neighbourFragmentEntry, end, entries, txn);
     
-        return entries;
+        return {entries, fragments};
     }
 }