feat: implement a generation counter for the CryptoStore lock

bindings/matrix-sdk-ffi/src/encryption_sync.rs

@@ -73,12 +73,6 @@ impl EncryptionSync {
             error!("Error when stopping the encryption sync: {err}");
         }
     }
-
-    pub fn reload_caches(&self) {
-        if let Err(err) = RUNTIME.block_on(self.sync.reload_caches()) {
-            error!("Error when reloading caches: {err}");
-        }
-    }
 }
 
 impl Client {

crates/matrix-sdk-crypto/src/machine.rs

@@ -67,8 +67,8 @@ use crate::{
     requests::{IncomingResponse, OutgoingRequest, UploadSigningKeysRequest},
     session_manager::{GroupSessionManager, SessionManager},
     store::{
-        Changes, DeviceChanges, DynCryptoStore, IdentityChanges, IntoCryptoStore, MemoryStore,
-        Result as StoreResult, SecretImportError, Store,
+        locks::LockStoreError, Changes, DeviceChanges, DynCryptoStore, IdentityChanges,
+        IntoCryptoStore, MemoryStore, Result as StoreResult, SecretImportError, Store,
     },
     types::{
         events::{
@@ -129,6 +129,18 @@ pub struct OlmMachineInner {
     /// A state machine that handles creating room key backups.
     #[cfg(feature = "backups_v1")]
     backup_machine: BackupMachine,
+    /// Latest "generation" of data known by the crypto store.
+    ///
+    /// This is a counter that only increments, set in the database (and can
+    /// wrap). It's incremented whenever some process acquires a lock for the
+    /// first time. *This assumes the crypto store lock is being held, to
+    /// avoid data races on writing to this value in the store*.
+    ///
+    /// The current process will maintain this value in local memory and in the
+    /// DB over time. Observing a different value than the one read in
+    /// memory, when reading from the store indicates that somebody else has
+    /// written into the database under our feet.
+    pub(crate) crypto_store_generation: Arc<Mutex<Option<u64>>>,
 }
 
 #[cfg(not(tarpaulin_include))]
@@ -142,6 +154,8 @@ impl std::fmt::Debug for OlmMachine {
 }
 
 impl OlmMachine {
+    const CURRENT_GENERATION_STORE_KEY: &str = "generation-counter";
+
     /// Create a new memory based OlmMachine.
     ///
     /// The created machine will keep the encryption keys only in memory and
@@ -212,6 +226,7 @@ impl OlmMachine {
             identity_manager,
             #[cfg(feature = "backups_v1")]
             backup_machine,
+            crypto_store_generation: Arc::new(Mutex::new(None)),
         });
 
         Self { inner }
@@ -1728,6 +1743,114 @@ impl OlmMachine {
     pub fn backup_machine(&self) -> &BackupMachine {
         &self.inner.backup_machine
     }
+
+    /// Syncs the database and in-memory generation counter.
+    ///
+    /// This requires that the crypto store lock has been acquired already.
+    pub async fn initialize_crypto_store_generation(&self) -> StoreResult<()> {
+        // Avoid reentrant initialization by taking the lock for the entire's function
+        // scope.
+        let mut gen_guard = self.inner.crypto_store_generation.lock().await;
+
+        let prev_generation =
+            self.inner.store.get_custom_value(Self::CURRENT_GENERATION_STORE_KEY).await?;
+
+        let gen = match prev_generation {
+            Some(val) => {
+                // There was a value in the store. We need to signal that we're a different
+                // process, so we don't just reuse the value but increment it.
+                u64::from_le_bytes(
+                    val.try_into().map_err(|_| LockStoreError::InvalidGenerationFormat)?,
+                )
+                .wrapping_add(1)
+            }
+            None => 0,
+        };
+
+        self.inner
+            .store
+            .set_custom_value(Self::CURRENT_GENERATION_STORE_KEY, gen.to_le_bytes().to_vec())
+            .await?;
+
+        *gen_guard = Some(gen);
+
+        Ok(())
+    }
+
+    /// If needs be, update the local and on-disk crypto store generation.
+    ///
+    /// Returns true whether another user has modified the internal generation
+    /// counter, and as such we've incremented and updated it in the
+    /// database.
+    ///
+    /// ## Requirements
+    ///
+    /// - This assumes that `initialize_crypto_store_generation` has been called
+    /// beforehand.
+    /// - This requires that the crypto store lock has been acquired.
+    pub async fn maintain_crypto_store_generation(&self) -> StoreResult<bool> {
+        let mut gen_guard = self.inner.crypto_store_generation.lock().await;
+
+        // The database value must be there:
+        // - either we could initialize beforehand, thus write into the database,
+        // - or we couldn't, and then another process was holding onto the database's
+        //   lock, thus
+        // has written a generation counter in there.
+        let actual_gen = self
+            .inner
+            .store
+            .get_custom_value(Self::CURRENT_GENERATION_STORE_KEY)
+            .await?
+            .ok_or(LockStoreError::MissingGeneration)?;
+
+        let actual_gen = u64::from_le_bytes(
+            actual_gen.try_into().map_err(|_| LockStoreError::InvalidGenerationFormat)?,
+        );
+
+        let expected_gen = match gen_guard.as_ref() {
+            Some(expected_gen) => {
+                if actual_gen == *expected_gen {
+                    return Ok(false);
+                }
+                // Increment the biggest, and store it everywhere.
+                actual_gen.max(*expected_gen).wrapping_add(1)
+            }
+            None => {
+                // Some other process hold onto the lock when initializing, so we must reload.
+                // Increment database value, and store it everywhere.
+                actual_gen.wrapping_add(1)
+            }
+        };
+
+        tracing::debug!(
+            "Crypto store generation mismatch: previously known was {:?}, actual is {:?}, next is {}",
+            *gen_guard,
+            actual_gen,
+            expected_gen
+        );
+
+        // Update known value.
+        *gen_guard = Some(expected_gen);
+
+        // Update value in database.
+        self.inner
+            .store
+            .set_custom_value(
+                Self::CURRENT_GENERATION_STORE_KEY,
+                expected_gen.to_le_bytes().to_vec(),
+            )
+            .await?;
+
+        Ok(true)
+    }
+
+    #[cfg(any(feature = "testing", test))]
+    /// Returns whether this `OlmMachine` is the same another one.
+    ///
+    /// Useful for testing purposes only.
+    pub fn same_as(&self, other: &OlmMachine) -> bool {
+        Arc::ptr_eq(&self.inner, &other.inner)
+    }
 }
 
 #[cfg(any(feature = "testing", test))]

crates/matrix-sdk-crypto/src/store/locks.rs

@@ -314,6 +314,15 @@ pub enum LockStoreError {
     /// Spent too long waiting for a database lock.
     #[error("a lock timed out")]
     LockTimeout,
+
+    /// The generation counter is missing, and should always be present.
+    #[error("missing generation counter in the store")]
+    MissingGeneration,
+
+    /// Unexpected format for the generation counter. Is someone tampering the
+    /// database?
+    #[error("invalid format of the generation counter")]
+    InvalidGenerationFormat,
 }
 
 #[cfg(test)]

crates/matrix-sdk-sqlite/Cargo.toml

@@ -9,6 +9,7 @@ rust-version = { workspace = true }
 
 [features]
 default = ["state-store"]
+testing = ["matrix-sdk-crypto?/testing"]
 
 bundled = ["rusqlite/bundled"]
 crypto-store = [

crates/matrix-sdk-ui/src/encryption_sync/mod.rs

@@ -213,19 +213,6 @@ impl EncryptionSync {
 
         Ok(())
     }
-
-    /// Request a reload of the internal caches used by this sync.
-    ///
-    /// This must be called every time the process running this loop was
-    /// suspended and got back into the foreground, and another process may have
-    /// written to the same underlying store (e.g. notification process vs
-    /// main process).
-    pub async fn reload_caches(&self) -> Result<(), Error> {
-        // Regenerate the crypto store caches first.
-        self.client.encryption().reload_caches().await.map_err(Error::ClientError)?;
-
-        Ok(())
-    }
 }
 
 /// Errors for the [`EncryptionSync`].

crates/matrix-sdk/Cargo.toml

@@ -17,7 +17,7 @@ rustdoc-args = ["--cfg", "docsrs"]
 
 [features]
 default = ["e2e-encryption", "automatic-room-key-forwarding", "sqlite", "native-tls"]
-testing = []
+testing = ["matrix-sdk-sqlite?/testing"]
 
 e2e-encryption = [
     "matrix-sdk-base/e2e-encryption",