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 <d@zerovolt.org>
This commit is contained in:
boxdot
2024-10-08 15:12:40 +02:00
committed by GitHub
parent 867d9c71fd
commit 4bcb9b7d9f

View File

@@ -642,28 +642,32 @@ impl OlmMachine {
&self,
reset: bool,
) -> StoreResult<CrossSigningBootstrapRequests> {
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.