diff --git a/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs b/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs index c7cf9f750..ba9c3d24b 100644 --- a/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs +++ b/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs @@ -34,9 +34,14 @@ use crate::{ #[allow(clippy::type_complexity)] #[derive(Debug)] pub struct MemoryStore { - media: StdRwLock)>>, - leases: StdRwLock>, - events: StdRwLock>, + inner: StdRwLock, +} + +#[derive(Debug)] +struct MemoryStoreInner { + media: RingBuffer<(OwnedMxcUri, String /* unique key */, Vec)>, + leases: HashMap, + events: RelationalLinkedChunk, } // SAFETY: `new_unchecked` is safe because 20 is not zero. @@ -45,9 +50,11 @@ const NUMBER_OF_MEDIAS: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(20) impl Default for MemoryStore { fn default() -> Self { Self { - media: StdRwLock::new(RingBuffer::new(NUMBER_OF_MEDIAS)), - leases: Default::default(), - events: StdRwLock::new(RelationalLinkedChunk::new()), + inner: StdRwLock::new(MemoryStoreInner { + media: RingBuffer::new(NUMBER_OF_MEDIAS), + leases: Default::default(), + events: RelationalLinkedChunk::new(), + }), } } } @@ -70,7 +77,9 @@ impl EventCacheStore for MemoryStore { key: &str, holder: &str, ) -> Result { - Ok(try_take_leased_lock(&self.leases, lease_duration_ms, key, holder)) + let mut inner = self.inner.write().unwrap(); + + Ok(try_take_leased_lock(&mut inner.leases, lease_duration_ms, key, holder)) } async fn handle_linked_chunk_updates( @@ -78,9 +87,9 @@ impl EventCacheStore for MemoryStore { room_id: &RoomId, updates: &[Update], ) -> Result<(), Self::Error> { - self.events.write().unwrap().apply_updates(room_id, updates); + let mut inner = self.inner.write().unwrap(); - Ok(()) + Ok(inner.events.apply_updates(updates)) } async fn add_media_content( @@ -90,8 +99,10 @@ impl EventCacheStore for MemoryStore { ) -> Result<()> { // Avoid duplication. Let's try to remove it first. self.remove_media_content(request).await?; + // Now, let's add it. - self.media.write().unwrap().push((request.uri().to_owned(), request.unique_key(), data)); + let mut inner = self.inner.write().unwrap(); + inner.media.push((request.uri().to_owned(), request.unique_key(), data)); Ok(()) } @@ -103,8 +114,10 @@ impl EventCacheStore for MemoryStore { ) -> Result<(), Self::Error> { let expected_key = from.unique_key(); - let mut medias = self.media.write().unwrap(); - if let Some((mxc, key, _)) = medias.iter_mut().find(|(_, key, _)| *key == expected_key) { + let mut inner = self.inner.write().unwrap(); + + if let Some((mxc, key, _)) = inner.media.iter_mut().find(|(_, key, _)| *key == expected_key) + { *mxc = to.uri().to_owned(); *key = to.unique_key(); } @@ -115,8 +128,9 @@ impl EventCacheStore for MemoryStore { async fn get_media_content(&self, request: &MediaRequestParameters) -> Result>> { let expected_key = request.unique_key(); - let media = self.media.read().unwrap(); - Ok(media.iter().find_map(|(_media_uri, media_key, media_content)| { + let inner = self.inner.write().unwrap(); + + Ok(inner.media.iter().find_map(|(_media_uri, media_key, media_content)| { (media_key == &expected_key).then(|| media_content.to_owned()) })) } @@ -124,23 +138,27 @@ impl EventCacheStore for MemoryStore { async fn remove_media_content(&self, request: &MediaRequestParameters) -> Result<()> { let expected_key = request.unique_key(); - let mut media = self.media.write().unwrap(); - let Some(index) = media + let mut inner = self.inner.write().unwrap(); + + let Some(index) = inner + .media .iter() .position(|(_media_uri, media_key, _media_content)| media_key == &expected_key) else { return Ok(()); }; - media.remove(index); + inner.media.remove(index); Ok(()) } async fn remove_media_content_for_uri(&self, uri: &MxcUri) -> Result<()> { - let mut media = self.media.write().unwrap(); + let mut inner = self.inner.write().unwrap(); + let expected_key = uri.to_owned(); - let positions = media + let positions = inner + .media .iter() .enumerate() .filter_map(|(position, (media_uri, _media_key, _media_content))| { @@ -150,7 +168,7 @@ impl EventCacheStore for MemoryStore { // Iterate in reverse-order so that positions stay valid after first removals. for position in positions.into_iter().rev() { - media.remove(position); + inner.media.remove(position); } Ok(()) diff --git a/crates/matrix-sdk-common/src/store_locks.rs b/crates/matrix-sdk-common/src/store_locks.rs index e31acc5ca..08af735ed 100644 --- a/crates/matrix-sdk-common/src/store_locks.rs +++ b/crates/matrix-sdk-common/src/store_locks.rs @@ -361,7 +361,7 @@ mod tests { impl TestStore { fn try_take_leased_lock(&self, lease_duration_ms: u32, key: &str, holder: &str) -> bool { - try_take_leased_lock(&self.leases, lease_duration_ms, key, holder) + try_take_leased_lock(&mut self.leases.write().unwrap(), lease_duration_ms, key, holder) } } @@ -502,12 +502,11 @@ mod tests { pub mod memory_store_helper { use std::{ collections::{hash_map::Entry, HashMap}, - sync::RwLock, time::{Duration, Instant}, }; pub fn try_take_leased_lock( - leases: &RwLock>, + leases: &mut HashMap, lease_duration_ms: u32, key: &str, holder: &str, @@ -515,7 +514,7 @@ pub mod memory_store_helper { let now = Instant::now(); let expiration = now + Duration::from_millis(lease_duration_ms.into()); - match leases.write().unwrap().entry(key.to_owned()) { + match leases.entry(key.to_owned()) { // There is an existing holder. Entry::Occupied(mut entry) => { let (current_holder, current_expiration) = entry.get_mut(); diff --git a/crates/matrix-sdk-crypto/src/store/memorystore.rs b/crates/matrix-sdk-crypto/src/store/memorystore.rs index 6b177ea26..8d5da3503 100644 --- a/crates/matrix-sdk-crypto/src/store/memorystore.rs +++ b/crates/matrix-sdk-crypto/src/store/memorystore.rs @@ -632,7 +632,7 @@ impl CryptoStore for MemoryStore { key: &str, holder: &str, ) -> Result { - Ok(try_take_leased_lock(&self.leases, lease_duration_ms, key, holder)) + Ok(try_take_leased_lock(&mut self.leases.write().unwrap(), lease_duration_ms, key, holder)) } }