Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: ensure the SessionStore is cleared when regenerating the OlmMachine #3338

Merged
merged 5 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
3 changes: 3 additions & 0 deletions crates/matrix-sdk-crypto/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ Breaking changes:

Additions:

- Expose new method `CryptoStore::clear_caches`.
([#3338](https://github.com/matrix-org/matrix-rust-sdk/pull/3338))

- Expose new method `OlmMachine::device_creation_time`.
([#3275](https://github.com/matrix-org/matrix-rust-sdk/pull/3275))

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
kegsay marked this conversation as resolved.
Show resolved Hide resolved
// 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
kegsay marked this conversation as resolved.
Show resolved Hide resolved
// 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
Loading