Skip to content

Commit

Permalink
bugfix: ensure the SessionStore is cleared when regenerating the OlmM…
Browse files Browse the repository at this point in the history
…achine

This fixes #3110
  • Loading branch information
kegsay committed Apr 18, 2024
1 parent f7329c7 commit 09955cf
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 1 deletion.
4 changes: 3 additions & 1 deletion crates/matrix-sdk-base/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,9 @@ impl BaseClient {
tracing::debug!("regenerating OlmMachine");
let session_meta = self.session_meta().ok_or(Error::OlmError(OlmError::MissingSession))?;

// Recreate it.
// Recreate the `OlmMachine` and wipe the in-memory cache in the store
// because we suspect it has stale data.
self.crypto_store.clear_caches().await;
let olm_machine = OlmMachine::with_store(
&session_meta.user_id,
&session_meta.device_id,
Expand Down
7 changes: 7 additions & 0 deletions crates/matrix-sdk-crypto/src/store/caches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ impl SessionStore {
Self::default()
}

/// Clear all entries in the session store.
///
/// This is intended to be used when regenerating olm machines.
pub fn clear(&self) {
self.entries.write().unwrap().clear()
}

/// Add a session to the store.
///
/// Returns true if the session was added, false if the session was
Expand Down
12 changes: 12 additions & 0 deletions crates/matrix-sdk-crypto/src/store/memorystore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ type Result<T> = std::result::Result<T, Infallible>;
impl CryptoStore for MemoryStore {
type Error = Infallible;

async fn clear_caches(&self) {
// no-op: it makes no sense to delete fields here as we would forget our
// identity, etc Effectively we have no caches as the fields
// *are* the underlying store. Calling this method only makes
// sense if there is some other layer (e.g disk) persistence
// happening.
}

async fn load_account(&self) -> Result<Option<Account>> {
Ok(self.account.read().unwrap().as_ref().map(|acc| acc.deep_clone()))
}
Expand Down Expand Up @@ -718,6 +726,10 @@ mod integration_tests {
impl CryptoStore for PersistentMemoryStore {
type Error = <MemoryStore as CryptoStore>::Error;

async fn clear_caches(&self) {
self.0.clear_caches().await
}

async fn load_account(&self) -> Result<Option<Account>, Self::Error> {
self.0.load_account().await
}
Expand Down
11 changes: 11 additions & 0 deletions crates/matrix-sdk-crypto/src/store/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,13 @@ pub trait CryptoStore: AsyncTraitDeps {

/// Load the next-batch token for a to-device query, if any.
async fn next_batch_token(&self) -> Result<Option<String>, Self::Error>;

/// Clear any in-memory caches because they may be out of sync with the
/// underlying data store.
///
/// If the store does not have any underlying persistence (e.g in-memory
/// store) then this should be a no-op.
async fn clear_caches(&self);
}

#[repr(transparent)]
Expand All @@ -320,6 +327,10 @@ impl<T: fmt::Debug> fmt::Debug for EraseCryptoStoreError<T> {
impl<T: CryptoStore> CryptoStore for EraseCryptoStoreError<T> {
type Error = CryptoStoreError;

async fn clear_caches(&self) {
self.0.clear_caches().await
}

async fn load_account(&self) -> Result<Option<Account>> {
self.0.load_account().await.map_err(Into::into)
}
Expand Down
6 changes: 6 additions & 0 deletions crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,12 @@ impl_crypto_store! {
}
}
}

async fn clear_caches(&self) {
self.session_cache.clear()
// We don't need to clear static_account as it only contains immutable data
// therefore cannot get out of sync with the underlying store.
}
}

impl Drop for IndexeddbCryptoStore {
Expand Down
7 changes: 7 additions & 0 deletions crates/matrix-sdk-sqlite/src/crypto_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,13 @@ impl SqliteObjectCryptoStoreExt for deadpool_sqlite::Object {}
impl CryptoStore for SqliteCryptoStore {
type Error = Error;

async fn clear_caches(&self) {
self.session_cache.clear()
// We don't need to clear static_account as it only contains immutable
// data therefore cannot get out of sync with the underlying
// store.
}

async fn load_account(&self) -> Result<Option<Account>> {
let conn = self.acquire().await?;
if let Some(pickle) = conn.get_kv("account").await? {
Expand Down

0 comments on commit 09955cf

Please sign in to comment.