timeline: sanitize usage of meta in the TimelineInnerStateTransaction

Before this patch, the meta field would be mutated, even when the transaction would be aborted. This changes the update scheme to meta
with the following:

- when creating the transaction, clone the meta (but keep the pointer location to the previous one)
- all the transaction's methods operate on the WIP meta
- when committing, replace the previous meta with the current one

Author: bnjbvr

crates/matrix-sdk-ui/src/timeline/event_handler.rs

@@ -249,7 +249,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
         state: &'a mut TimelineInnerStateTransaction<'o>,
         ctx: TimelineEventContext,
     ) -> Self {
-        let TimelineInnerStateTransaction { items, meta } = state;
+        let TimelineInnerStateTransaction { items, meta, .. } = state;
         Self { items, meta, ctx, result: HandleEventResult::default() }
     }
 

crates/matrix-sdk-ui/src/timeline/inner/state.rs

@@ -407,7 +407,8 @@ impl TimelineInnerState {
 
     fn transaction(&mut self) -> TimelineInnerStateTransaction<'_> {
         let items = ManuallyDrop::new(self.items.transaction());
-        TimelineInnerStateTransaction { items, meta: &mut self.meta }
+        let meta = ManuallyDrop::new(self.meta.clone());
+        TimelineInnerStateTransaction { items, previous_meta: &mut self.meta, meta }
     }
 }
 
@@ -427,7 +428,14 @@ impl DerefMut for TimelineInnerState {
 
 pub(in crate::timeline) struct TimelineInnerStateTransaction<'a> {
     pub items: ManuallyDrop<ObservableVectorTransaction<'a, Arc<TimelineItem>>>,
-    pub meta: &'a mut TimelineInnerMetadata,
+
+    /// A clone of the previous meta, that we're operating on during the
+    /// transaction, and that will be committed to the previous meta location in
+    /// [`Self::commit`].
+    pub meta: ManuallyDrop<TimelineInnerMetadata>,
+
+    /// Pointer to the previous meta, only used during [`Self::commit`].
+    previous_meta: &'a mut TimelineInnerMetadata,
 }
 
 impl TimelineInnerStateTransaction<'_> {
@@ -630,14 +638,17 @@ impl TimelineInnerStateTransaction<'_> {
     }
 
     fn commit(mut self) {
-        let Self {
-            items,
-            // meta is just a reference, does not any dropping
-            meta: _,
-        } = &mut self;
+        let Self { items, previous_meta, meta } = &mut self;
+
+        // Safety: self is forgotten to avoid double free from drop.
+        let meta = unsafe { ManuallyDrop::take(meta) };
 
-        // Safety: self is forgotten to avoid double free from drop
+        // Replace the pointer to the previous meta with the new one.
+        **previous_meta = meta;
+
+        // Safety: self is forgotten to avoid double free from drop.
         let items = unsafe { ManuallyDrop::take(items) };
+
         mem::forget(self);
 
         items.commit();
@@ -701,7 +712,7 @@ impl Drop for TimelineInnerStateTransaction<'_> {
     }
 }
 
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub(in crate::timeline) struct TimelineInnerMetadata {
     /// List of all the events as received in the timeline, even the ones that
     /// are discarded in the timeline items.

crates/matrix-sdk-ui/src/timeline/polls.rs

@@ -141,7 +141,7 @@ impl From<PollState> for NewUnstablePollStartEventContent {
 
 /// Acts as a cache for poll response and poll end events handled before their
 /// start event has been handled.
-#[derive(Debug, Default)]
+#[derive(Clone, Debug, Default)]
 pub(super) struct PollPendingEvents {
     pub(super) pending_poll_responses: HashMap<OwnedEventId, Vec<ResponseData>>,
     pub(super) pending_poll_ends: HashMap<OwnedEventId, MilliSecondsSinceUnixEpoch>,

crates/matrix-sdk-ui/src/timeline/reactions.rs

@@ -33,7 +33,7 @@ pub struct ReactionSenderData {
     pub timestamp: MilliSecondsSinceUnixEpoch,
 }
 
-#[derive(Debug, Default)]
+#[derive(Clone, Debug, Default)]
 pub(super) struct Reactions {
     /// Reaction event / txn ID => sender and reaction data.
     pub(super) map: HashMap<EventItemIdentifier, (ReactionSenderData, Annotation)>,

crates/matrix-sdk-ui/src/timeline/read_receipts.rs

@@ -370,10 +370,11 @@ impl TimelineInnerStateTransaction<'_> {
                         receipt: &receipt,
                     };
 
-                    self.meta.read_receipts.maybe_update_read_receipt(
+                    let meta = &mut *self.meta;
+                    meta.read_receipts.maybe_update_read_receipt(
                         full_receipt,
                         is_own_user_id,
-                        &self.meta.all_events,
+                        &meta.all_events,
                         &mut self.items,
                     );
                 }
@@ -404,10 +405,12 @@ impl TimelineInnerStateTransaction<'_> {
                 receipt_type: ReceiptType::Read,
                 receipt: &receipt,
             };
-            self.meta.read_receipts.maybe_update_read_receipt(
+
+            let meta = &mut *self.meta;
+            meta.read_receipts.maybe_update_read_receipt(
                 full_receipt,
                 user_id == own_user_id,
-                &self.meta.all_events,
+                &meta.all_events,
                 &mut self.items,
             );
         }
@@ -433,10 +436,11 @@ impl TimelineInnerStateTransaction<'_> {
         let full_receipt =
             FullReceipt { event_id, user_id, receipt_type: ReceiptType::Read, receipt: &receipt };
 
-        self.meta.read_receipts.maybe_update_read_receipt(
+        let meta = &mut *self.meta;
+        meta.read_receipts.maybe_update_read_receipt(
             full_receipt,
             is_own_event,
-            &self.meta.all_events,
+            &meta.all_events,
             &mut self.items,
         );
     }