From c8998dc7eca5323d0e06b2d8a91d776c4c300f9d Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Fri, 15 Nov 2024 11:11:26 +0100 Subject: [PATCH] build: replace `async-lock` with `tokio` locks in `core-crypto` This is experimental work, the purpose of which is to see how hard it would be to just swap out the lock implementations. If all had gone well, then it would have been worthwhile to make this change for the `map` and `try_map` that `tokio`'s lock guards have, which could have improved the ergonomics of some of our internal methods. Unfortunately, the difficulty was more than trivial. While it took only a few minutes of mechanical work to swap out the imports and uses, this causes a handful of tests to fail. So it's not really worth proceeding further with this experiment until we've actually prioritized it. --- Cargo.lock | 6 +++--- Cargo.toml | 5 +---- crypto/Cargo.toml | 2 +- crypto/src/context.rs | 12 ++++++------ crypto/src/group_store.rs | 10 +++++----- crypto/src/lib.rs | 2 +- crypto/src/mls/client/mod.rs | 2 +- crypto/src/mls/mod.rs | 2 +- 8 files changed, 19 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a2175ac988..edb0e6209c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -895,7 +895,6 @@ name = "core-crypto" version = "1.1.0" dependencies = [ "async-fs", - "async-lock", "async-recursion", "async-std", "async-trait", @@ -938,6 +937,7 @@ dependencies = [ "thiserror", "time", "tls_codec", + "tokio", "uniffi", "url", "uuid", @@ -4660,9 +4660,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.41.0" +version = "1.41.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "145f3413504347a2be84393cc8a7d2fb4d863b375909ea59f2158261aa258bbb" +checksum = "22cfb5bee7a6a52939ca9224d6ac897bb669134078daa8735560897f69de4d33" dependencies = [ "backtrace", "bytes", diff --git a/Cargo.toml b/Cargo.toml index b5f6b2cd18..3e83e0973f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,6 +46,7 @@ sha2 = "0.10" strum = { version = "0.26", features = ["derive"] } thiserror = "1.0" tls_codec = "0.4.1" +tokio = { version = "1.41.1", features = ["sync"] } uniffi = "0.28" url = "2.5" uuid = "1.11" @@ -72,7 +73,3 @@ opt-level = "s" # ! This will cause the FFI to stop working because UniFFI stores the Rust <-> cdylib mapping # ! in the `.strtab` section of the executable. Stripping this causes everything to stop functioning. strip = false -# panic = "abort" - -[profile.dev.package.backtrace] -opt-level = 3 diff --git a/crypto/Cargo.toml b/crypto/Cargo.toml index 200cdb6cc5..ce7cc90c8a 100644 --- a/crypto/Cargo.toml +++ b/crypto/Cargo.toml @@ -40,7 +40,6 @@ serde.workspace = true serde_json.workspace = true url.workspace = true async-trait.workspace = true -async-lock.workspace = true schnellru.workspace = true zeroize.workspace = true wire-e2e-identity.workspace = true @@ -55,6 +54,7 @@ base64.workspace = true log.workspace = true proteus-wasm = { workspace = true, features = ["hazmat"], optional = true } proteus-traits = { workspace = true, optional = true } +tokio.workspace = true [target.'cfg(not(target_family = "wasm"))'.dependencies] sysinfo = { version = "0.32", default-features = false, features = ["apple-app-store", "system"] } diff --git a/crypto/src/context.rs b/crypto/src/context.rs index 37317dabbc..509db2abf0 100644 --- a/crypto/src/context.rs +++ b/crypto/src/context.rs @@ -9,12 +9,12 @@ use crate::{ prelude::{Client, MlsConversation}, CoreCrypto, CoreCryptoCallbacks, CryptoError, CryptoResult, }; -use async_lock::{Mutex, RwLock, RwLockReadGuardArc, RwLockWriteGuardArc}; use core_crypto_keystore::connection::FetchFromDatabase; use core_crypto_keystore::entities::ConsumerData; use core_crypto_keystore::CryptoKeystoreError; use mls_crypto_provider::{CryptoKeystore, MlsCryptoProvider}; use std::{ops::Deref, sync::Arc}; +use tokio::sync::{Mutex, OwnedRwLockReadGuard, OwnedRwLockWriteGuard, RwLock}; /// This struct provides transactional support for Core Crypto. /// @@ -90,9 +90,9 @@ impl CentralContext { pub(crate) async fn callbacks( &self, - ) -> CryptoResult>>> { + ) -> CryptoResult>>> { match self.state.read().await.deref() { - ContextState::Valid { callbacks, .. } => Ok(callbacks.read_arc().await), + ContextState::Valid { callbacks, .. } => Ok(callbacks.clone().read_owned().await), ContextState::Invalid => Err(CryptoError::InvalidContext), } } @@ -104,7 +104,7 @@ impl CentralContext { ) -> CryptoResult<()> { match self.state.read().await.deref() { ContextState::Valid { callbacks: cbs, .. } => { - *cbs.write_arc().await = callbacks; + *cbs.clone().write_owned().await = callbacks; Ok(()) } ContextState::Invalid => Err(CryptoError::InvalidContext), @@ -126,9 +126,9 @@ impl CentralContext { } } - pub(crate) async fn mls_groups(&self) -> CryptoResult>> { + pub(crate) async fn mls_groups(&self) -> CryptoResult>> { match self.state.read().await.deref() { - ContextState::Valid { mls_groups, .. } => Ok(mls_groups.write_arc().await), + ContextState::Valid { mls_groups, .. } => Ok(mls_groups.clone().write_owned().await), ContextState::Invalid => Err(CryptoError::InvalidContext), } } diff --git a/crypto/src/group_store.rs b/crypto/src/group_store.rs index 2e4e61617c..95d70ed702 100644 --- a/crypto/src/group_store.rs +++ b/crypto/src/group_store.rs @@ -123,7 +123,7 @@ impl GroupStoreEntity for crate::proteus::ProteusConversationSession { } } -pub(crate) type GroupStoreValue = std::sync::Arc>; +pub(crate) type GroupStoreValue = std::sync::Arc>; pub(crate) type LruMap = schnellru::LruMap, GroupStoreValue, HybridMemoryLimiter>; @@ -206,7 +206,7 @@ impl GroupStore { // Not in store, fetch the thing in the keystore let mut value = V::fetch_from_id(k, identity, keystore).await?; if let Some(value) = value.take() { - let value_to_insert = std::sync::Arc::new(async_lock::RwLock::new(value)); + let value_to_insert = std::sync::Arc::new(tokio::sync::RwLock::new(value)); self.insert_prepped(k.to_vec(), value_to_insert.clone()); Ok(Some(value_to_insert)) @@ -235,7 +235,7 @@ impl GroupStore { .into_iter() .map(|g| { let id = g.id().to_vec(); - let to_insert = std::sync::Arc::new(async_lock::RwLock::new(g)); + let to_insert = std::sync::Arc::new(tokio::sync::RwLock::new(g)); self.insert_prepped(id, to_insert.clone()); to_insert }) @@ -248,12 +248,12 @@ impl GroupStore { } pub(crate) fn insert(&mut self, k: Vec, entity: V) { - let value_to_insert = std::sync::Arc::new(async_lock::RwLock::new(entity)); + let value_to_insert = std::sync::Arc::new(tokio::sync::RwLock::new(entity)); self.insert_prepped(k, value_to_insert) } pub(crate) fn try_insert(&mut self, k: Vec, entity: V) -> Result<(), V> { - let value_to_insert = std::sync::Arc::new(async_lock::RwLock::new(entity)); + let value_to_insert = std::sync::Arc::new(tokio::sync::RwLock::new(entity)); if self.0.insert(k, value_to_insert.clone()) { Ok(()) diff --git a/crypto/src/lib.rs b/crypto/src/lib.rs index 78a93b3169..d62f809b63 100644 --- a/crypto/src/lib.rs +++ b/crypto/src/lib.rs @@ -23,10 +23,10 @@ #![deny(missing_docs)] #![allow(clippy::single_component_path_imports)] -use async_lock::Mutex; #[cfg(test)] pub use core_crypto_attributes::{dispotent, durable, idempotent}; use std::sync::Arc; +use tokio::sync::Mutex; pub use self::error::*; diff --git a/crypto/src/mls/client/mod.rs b/crypto/src/mls/client/mod.rs index bf2cbb6ab5..dcdfa6af09 100644 --- a/crypto/src/mls/client/mod.rs +++ b/crypto/src/mls/client/mod.rs @@ -27,7 +27,6 @@ use crate::{ CryptoError, CryptoResult, MlsCiphersuite, MlsCredentialType, MlsError, }, }; -use async_lock::RwLock; use core_crypto_keystore::{connection::FetchFromDatabase, Connection, CryptoKeystoreError}; use log::debug; use openmls::prelude::{Credential, CredentialType}; @@ -37,6 +36,7 @@ use std::collections::HashSet; use std::ops::{Deref, DerefMut}; use std::sync::Arc; use tls_codec::{Deserialize, Serialize}; +use tokio::sync::RwLock; use core_crypto_keystore::entities::{EntityFindParams, MlsCredential, MlsSignatureKeyPair}; use identities::ClientIdentities; diff --git a/crypto/src/mls/mod.rs b/crypto/src/mls/mod.rs index f277a28657..04c32420d9 100644 --- a/crypto/src/mls/mod.rs +++ b/crypto/src/mls/mod.rs @@ -1,7 +1,7 @@ use std::sync::Arc; -use async_lock::RwLock; use log::trace; +use tokio::sync::RwLock; use crate::prelude::{ identifier::ClientIdentifier, key_package::INITIAL_KEYING_MATERIAL_COUNT, Client, ClientId, ConversationId,