From 4bcb9b7d9ff6f0fa36f730d689b241f93bd573c9 Mon Sep 17 00:00:00 2001 From: boxdot Date: Tue, 8 Oct 2024 15:12:40 +0200 Subject: [PATCH] fix: Fix a deadlock between `bootstrap_cross_signing` and `sync` (#4060) `bootstrap_cross_signing` holds a lock on the private identity. In case a new identity is created, it will try to acquire a lock on `account`. The latter is locked by `sync`, which tries to acquire a lock on the private identity. Note that the `bootstrap_cross_signing` call is executed in a separate task e.g. in `restore_session`. In particular, this task and `sync` both race to acquire locks described above. Signed-off-by: boxdot --- crates/matrix-sdk-crypto/src/machine/mod.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/machine/mod.rs b/crates/matrix-sdk-crypto/src/machine/mod.rs index 949ca1e56..80b2c1d44 100644 --- a/crates/matrix-sdk-crypto/src/machine/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/mod.rs @@ -642,28 +642,32 @@ impl OlmMachine { &self, reset: bool, ) -> StoreResult { - let mut identity = self.inner.user_identity.lock().await; + // Don't hold the lock, otherwise we might deadlock in + // `bootstrap_cross_signing()` on `account` if a sync task is already + // running (which locks `account`), or we will deadlock + // in `upload_device_keys()` which locks private identity again. + let identity = self.inner.user_identity.lock().await.clone(); let (upload_signing_keys_req, upload_signatures_req) = if reset || identity.is_empty().await { info!("Creating new cross signing identity"); - let (new_identity, upload_signing_keys_req, upload_signatures_req) = { + let (identity, upload_signing_keys_req, upload_signatures_req) = { let cache = self.inner.store.cache().await?; let account = cache.account().await?; account.bootstrap_cross_signing().await }; - *identity = new_identity; - let public = identity.to_public_identity().await.expect( "Couldn't create a public version of the identity from a new private identity", ); + *self.inner.user_identity.lock().await = identity.clone(); + self.store() .save_changes(Changes { identities: IdentityChanges { new: vec![public.into()], ..Default::default() }, - private_identity: Some(identity.clone()), + private_identity: Some(identity), ..Default::default() }) .await?; @@ -682,11 +686,6 @@ impl OlmMachine { (upload_signing_keys_req, upload_signatures_req) }; - // `upload_device_keys()` will attempt to sign the device keys using this - // `identity`, it will attempt to acquire the lock, so we need to drop - // here to avoid a deadlock. - drop(identity); - // If there are any *device* keys to upload (i.e. the account isn't shared), // upload them before we upload the signatures, since the signatures may // reference keys to be uploaded.