From e37cfe603628b744391bae1bbefaee85da6bd70b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 28 Sep 2023 12:11:12 +0200 Subject: [PATCH] Don't use a static for the mark_room_as_dm lock Using a static lock would prevent two Client instances to call the method at the same time. We're going to assume that people won't have multiple Client instances for the same user, in which case a static lock would have been helpful. Co-authored-by: Jonas Platte --- crates/matrix-sdk/src/account.rs | 5 +---- crates/matrix-sdk/src/client/mod.rs | 21 ++++++++++++++++++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/crates/matrix-sdk/src/account.rs b/crates/matrix-sdk/src/account.rs index 8ea7e7826..eae71a96d 100644 --- a/crates/matrix-sdk/src/account.rs +++ b/crates/matrix-sdk/src/account.rs @@ -47,7 +47,6 @@ use ruma::{ ClientSecret, MxcUri, OwnedMxcUri, OwnedUserId, RoomId, SessionId, UInt, UserId, }; use serde::Deserialize; -use tokio::sync::Mutex; use tracing::error; use crate::{config::RequestConfig, Client, Error, HttpError, Result}; @@ -821,9 +820,7 @@ impl Account { // To prevent multiple calls to this method trying to update the map of DMs same // time, and thus trampling on each other we introduce a lock which acts // as a semaphore. - static LOCK: Mutex<()> = Mutex::const_new(()); - - let _guard = LOCK.lock().await; + let _guard = self.client.locks().mark_as_dm_lock.lock().await; // Now we need to mark the room as a DM for ourselves, we fetch the // existing `m.direct` event and append the room to the list of DMs we diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index 15676f069..819021468 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -65,9 +65,7 @@ use ruma::{ ServerName, UInt, UserId, }; use serde::de::DeserializeOwned; -#[cfg(feature = "e2e-encryption")] -use tokio::sync::Mutex; -use tokio::sync::{broadcast, OnceCell, RwLock, RwLockReadGuard}; +use tokio::sync::{broadcast, Mutex, OnceCell, RwLock, RwLockReadGuard}; use tracing::{debug, error, instrument, trace, Instrument, Span}; use url::Url; @@ -144,6 +142,14 @@ pub struct Client { pub(crate) inner: Arc, } +#[derive(Default)] +pub(crate) struct ClientLocks { + /// Lock ensuring that only a single room may be marked as a DM at once. + /// Look at the [`Room::mark_as_dm()`] method for a more detailed + /// explanation. + pub(crate) mark_as_dm_lock: Mutex<()>, +} + pub(crate) struct ClientInner { /// All the data related to authentication and authorization. pub(crate) auth_ctx: Arc, @@ -159,6 +165,10 @@ pub(crate) struct ClientInner { base_client: BaseClient, /// The Matrix versions the server supports (well-known ones only) server_versions: OnceCell>, + /// Collection of locks individual client methods might want to use, either + /// to ensure that only a single call to a method happens at once or to + /// deduplicate multiple calls to a method. + locks: ClientLocks, /// Handler making sure we only have one group session sharing request in /// flight per room. #[cfg(feature = "e2e-encryption")] @@ -238,6 +248,7 @@ impl ClientInner { sliding_sync_proxy: StdRwLock::new(sliding_sync_proxy), http_client, base_client, + locks: Default::default(), server_versions: OnceCell::new_with(server_versions), #[cfg(feature = "e2e-encryption")] group_session_deduplicated_handler: Default::default(), @@ -296,6 +307,10 @@ impl Client { &self.inner.base_client } + pub(crate) fn locks(&self) -> &ClientLocks { + &self.inner.locks + } + /// Change the homeserver URL used by this client. /// /// # Arguments