timeline: get rid of ManuallyDrop in the TimelineInnerStateTransaction

Author: bnjbvr

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

@@ -12,12 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::{
-    collections::VecDeque,
-    future::Future,
-    mem::{self, ManuallyDrop},
-    sync::Arc,
-};
+use std::{collections::VecDeque, future::Future, sync::Arc};
 
 use eyeball_im::{ObservableVector, ObservableVectorTransaction, ObservableVectorTransactionEntry};
 use indexmap::IndexMap;
@@ -403,19 +398,21 @@ impl TimelineInnerState {
     }
 
     fn transaction(&mut self) -> TimelineInnerStateTransaction<'_> {
-        let items = ManuallyDrop::new(self.items.transaction());
-        let meta = ManuallyDrop::new(self.meta.clone());
+        let items = self.items.transaction();
+        let meta = self.meta.clone();
         TimelineInnerStateTransaction { items, previous_meta: &mut self.meta, meta }
     }
 }
 
 pub(in crate::timeline) struct TimelineInnerStateTransaction<'a> {
-    pub items: ManuallyDrop<ObservableVectorTransaction<'a, Arc<TimelineItem>>>,
+    /// A vector transaction over the items themselves. Holds temporary state
+    /// until committed.
+    pub items: ObservableVectorTransaction<'a, Arc<TimelineItem>>,
 
     /// 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>,
+    pub meta: TimelineInnerMetadata,
 
     /// Pointer to the previous meta, only used during [`Self::commit`].
     previous_meta: &'a mut TimelineInnerMetadata,
@@ -624,19 +621,11 @@ impl TimelineInnerStateTransaction<'_> {
         self.meta.update_read_marker(&mut self.items);
     }
 
-    fn commit(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) };
+    fn commit(self) {
+        let Self { items, previous_meta, meta } = self;
 
         // 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);
+        *previous_meta = meta;
 
         items.commit();
     }
@@ -688,17 +677,6 @@ impl TimelineInnerStateTransaction<'_> {
     }
 }
 
-impl Drop for TimelineInnerStateTransaction<'_> {
-    fn drop(&mut self) {
-        warn!("timeline state transaction cancelled");
-        // Safety: self.items is not touched anymore, the only other place
-        // dropping is Self::commit which makes sure to skip this Drop impl.
-        unsafe {
-            ManuallyDrop::drop(&mut self.items);
-        }
-    }
-}
-
 #[derive(Clone, Debug)]
 pub(in crate::timeline) struct TimelineInnerMetadata {
     /// List of all the events as received in the timeline, even the ones that

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

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