Skip to content

Commit 2a8befa

Browse files
HywanHywan
committed
feat(common): Add a cross-process lock generation.
This patch adds `CrossProcessLockGeneration`. A lock generation is an integer incremented each time the lock is taken by another holder. If the generation changes, it means the lock is _poisoned_. This _poisoning_ aspect is going to be expanded in the next patches. This patch focuses on the introduction of this _generation_. The `CrossProcessLock::try_lock_once` method, and the `TryLock::try_lock` method, both returns a `Option<CrossProcessLockGeneration>` instead of a `bool`: `true` is replaced by `Some(_)`, `false` by `None`.
1 parent f12ee86 commit 2a8befa

File tree

20 files changed

+248
-120
lines changed

20 files changed

+248
-120
lines changed

‎crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,57 +1304,57 @@ macro_rules! event_cache_store_integration_tests_time {
13041304
let store = get_event_cache_store().await.unwrap().into_event_cache_store();
13051305

13061306
let acquired0 = store.try_take_leased_lock(0, "key", "alice").await.unwrap();
1307-
assert!(acquired0);
1307+
assert_eq!(acquired0, Some(1)); // first lock generation
13081308

13091309
// Should extend the lease automatically (same holder).
13101310
let acquired2 = store.try_take_leased_lock(300, "key", "alice").await.unwrap();
1311-
assert!(acquired2);
1311+
assert_eq!(acquired2, Some(1)); // same lock generation
13121312

13131313
// Should extend the lease automatically (same holder + time is ok).
13141314
let acquired3 = store.try_take_leased_lock(300, "key", "alice").await.unwrap();
1315-
assert!(acquired3);
1315+
assert_eq!(acquired3, Some(1)); // same lock generation
13161316

13171317
// Another attempt at taking the lock should fail, because it's taken.
13181318
let acquired4 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
1319-
assert!(!acquired4);
1319+
assert!(acquired4.is_none()); // not acquired
13201320

13211321
// Even if we insist.
13221322
let acquired5 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
1323-
assert!(!acquired5);
1323+
assert!(acquired5.is_none()); // not acquired
13241324

13251325
// That's a nice test we got here, go take a little nap.
13261326
sleep(Duration::from_millis(50)).await;
13271327

13281328
// Still too early.
13291329
let acquired55 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
1330-
assert!(!acquired55);
1330+
assert!(acquired55.is_none()); // not acquired
13311331

13321332
// Ok you can take another nap then.
13331333
sleep(Duration::from_millis(250)).await;
13341334

13351335
// At some point, we do get the lock.
13361336
let acquired6 = store.try_take_leased_lock(0, "key", "bob").await.unwrap();
1337-
assert!(acquired6);
1337+
assert_eq!(acquired6, Some(2)); // new lock generation!
13381338

13391339
sleep(Duration::from_millis(1)).await;
13401340

13411341
// The other gets it almost immediately too.
13421342
let acquired7 = store.try_take_leased_lock(0, "key", "alice").await.unwrap();
1343-
assert!(acquired7);
1343+
assert_eq!(acquired7, Some(3)); // new lock generation!
13441344

13451345
sleep(Duration::from_millis(1)).await;
13461346

1347-
// But when we take a longer lease...
1347+
// But when we take a longer lease…
13481348
let acquired8 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
1349-
assert!(acquired8);
1349+
assert_eq!(acquired8, Some(4)); // new lock generation!
13501350

13511351
// It blocks the other user.
13521352
let acquired9 = store.try_take_leased_lock(300, "key", "alice").await.unwrap();
1353-
assert!(!acquired9);
1353+
assert!(acquired9.is_none()); // not acquired
13541354

13551355
// We can hold onto our lease.
13561356
let acquired10 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
1357-
assert!(acquired10);
1357+
assert_eq!(acquired10, Some(4)); // same lock generation
13581358
}
13591359
}
13601360
};

‎crates/matrix-sdk-base/src/event_cache/store/memory_store.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,16 @@ use std::{
1919

2020
use async_trait::async_trait;
2121
use matrix_sdk_common::{
22-
cross_process_lock::memory_store_helper::try_take_leased_lock,
22+
cross_process_lock::{
23+
CrossProcessLockGeneration,
24+
memory_store_helper::{Lease, try_take_leased_lock},
25+
},
2326
linked_chunk::{
2427
ChunkIdentifier, ChunkIdentifierGenerator, ChunkMetadata, LinkedChunkId, Position,
2528
RawChunk, Update, relational::RelationalLinkedChunk,
2629
},
2730
};
28-
use ruma::{EventId, OwnedEventId, RoomId, events::relation::RelationType, time::Instant};
31+
use ruma::{EventId, OwnedEventId, RoomId, events::relation::RelationType};
2932
use tracing::error;
3033

3134
use super::{
@@ -43,7 +46,7 @@ pub struct MemoryStore {
4346

4447
#[derive(Debug)]
4548
struct MemoryStoreInner {
46-
leases: HashMap<String, (String, Instant)>,
49+
leases: HashMap<String, Lease>,
4750
events: RelationalLinkedChunk<OwnedEventId, Event, Gap>,
4851
}
4952

@@ -75,7 +78,7 @@ impl EventCacheStore for MemoryStore {
7578
lease_duration_ms: u32,
7679
key: &str,
7780
holder: &str,
78-
) -> Result<bool, Self::Error> {
81+
) -> Result<Option<CrossProcessLockGeneration>, Self::Error> {
7982
let mut inner = self.inner.write().unwrap();
8083

8184
Ok(try_take_leased_lock(&mut inner.leases, lease_duration_ms, key, holder))

‎crates/matrix-sdk-base/src/event_cache/store/mod.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ mod memory_store;
2828
mod traits;
2929

3030
use matrix_sdk_common::cross_process_lock::{
31-
CrossProcessLock, CrossProcessLockError, CrossProcessLockGuard, TryLock,
31+
CrossProcessLock, CrossProcessLockError, CrossProcessLockGeneration, CrossProcessLockGuard,
32+
TryLock,
3233
};
3334
pub use matrix_sdk_store_encryption::Error as StoreEncryptionError;
3435
use ruma::{
@@ -192,7 +193,7 @@ impl TryLock for LockableEventCacheStore {
192193
lease_duration_ms: u32,
193194
key: &str,
194195
holder: &str,
195-
) -> std::result::Result<bool, Self::LockError> {
196+
) -> std::result::Result<Option<CrossProcessLockGeneration>, Self::LockError> {
196197
self.0.try_take_leased_lock(lease_duration_ms, key, holder).await
197198
}
198199
}

‎crates/matrix-sdk-base/src/event_cache/store/traits.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use std::{fmt, sync::Arc};
1717
use async_trait::async_trait;
1818
use matrix_sdk_common::{
1919
AsyncTraitDeps,
20+
cross_process_lock::CrossProcessLockGeneration,
2021
linked_chunk::{
2122
ChunkIdentifier, ChunkIdentifierGenerator, ChunkMetadata, LinkedChunkId, Position,
2223
RawChunk, Update,
@@ -46,7 +47,7 @@ pub trait EventCacheStore: AsyncTraitDeps {
4647
lease_duration_ms: u32,
4748
key: &str,
4849
holder: &str,
49-
) -> Result<bool, Self::Error>;
50+
) -> Result<Option<CrossProcessLockGeneration>, Self::Error>;
5051

5152
/// An [`Update`] reflects an operation that has happened inside a linked
5253
/// chunk. The linked chunk is used by the event cache to store the events
@@ -191,7 +192,7 @@ impl<T: EventCacheStore> EventCacheStore for EraseEventCacheStoreError<T> {
191192
lease_duration_ms: u32,
192193
key: &str,
193194
holder: &str,
194-
) -> Result<bool, Self::Error> {
195+
) -> Result<Option<CrossProcessLockGeneration>, Self::Error> {
195196
self.0.try_take_leased_lock(lease_duration_ms, key, holder).await.map_err(Into::into)
196197
}
197198

‎crates/matrix-sdk-base/src/media/store/integration_tests.rs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,57 +1337,57 @@ macro_rules! media_store_integration_tests_time {
13371337
let store = get_media_store().await.unwrap();
13381338

13391339
let acquired0 = store.try_take_leased_lock(0, "key", "alice").await.unwrap();
1340-
assert!(acquired0);
1340+
assert_eq!(acquired0, Some(1)); // first lock generation
13411341

13421342
// Should extend the lease automatically (same holder).
13431343
let acquired2 = store.try_take_leased_lock(300, "key", "alice").await.unwrap();
1344-
assert!(acquired2);
1344+
assert_eq!(acquired2, Some(1)); // same lock generation
13451345

13461346
// Should extend the lease automatically (same holder + time is ok).
13471347
let acquired3 = store.try_take_leased_lock(300, "key", "alice").await.unwrap();
1348-
assert!(acquired3);
1348+
assert_eq!(acquired3, Some(1)); // same lock generation
13491349

13501350
// Another attempt at taking the lock should fail, because it's taken.
13511351
let acquired4 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
1352-
assert!(!acquired4);
1352+
assert!(acquired4.is_none()); // not acquired
13531353

13541354
// Even if we insist.
13551355
let acquired5 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
1356-
assert!(!acquired5);
1356+
assert!(acquired5.is_none()); // not acquired
13571357

13581358
// That's a nice test we got here, go take a little nap.
13591359
sleep(Duration::from_millis(50)).await;
13601360

13611361
// Still too early.
13621362
let acquired55 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
1363-
assert!(!acquired55);
1363+
assert!(acquired55.is_none()); // not acquired
13641364

13651365
// Ok you can take another nap then.
13661366
sleep(Duration::from_millis(250)).await;
13671367

13681368
// At some point, we do get the lock.
13691369
let acquired6 = store.try_take_leased_lock(0, "key", "bob").await.unwrap();
1370-
assert!(acquired6);
1370+
assert_eq!(acquired6, Some(2)); // new lock generation!
13711371

13721372
sleep(Duration::from_millis(1)).await;
13731373

13741374
// The other gets it almost immediately too.
13751375
let acquired7 = store.try_take_leased_lock(0, "key", "alice").await.unwrap();
1376-
assert!(acquired7);
1376+
assert_eq!(acquired7, Some(3)); // new lock generation!
13771377

13781378
sleep(Duration::from_millis(1)).await;
13791379

1380-
// But when we take a longer lease...
1380+
// But when we take a longer lease…
13811381
let acquired8 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
1382-
assert!(acquired8);
1382+
assert_eq!(acquired8, Some(4)); // new lock generation!
13831383

13841384
// It blocks the other user.
13851385
let acquired9 = store.try_take_leased_lock(300, "key", "alice").await.unwrap();
1386-
assert!(!acquired9);
1386+
assert!(acquired9.is_none()); // not acquired
13871387

13881388
// We can hold onto our lease.
13891389
let acquired10 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
1390-
assert!(acquired10);
1390+
assert_eq!(acquired10, Some(4)); // same lock generation
13911391
}
13921392
}
13931393
};

‎crates/matrix-sdk-base/src/media/store/memory_store.rs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,13 @@ use std::{
2020

2121
use async_trait::async_trait;
2222
use matrix_sdk_common::{
23-
cross_process_lock::memory_store_helper::try_take_leased_lock, ring_buffer::RingBuffer,
24-
};
25-
use ruma::{
26-
MxcUri, OwnedMxcUri,
27-
time::{Instant, SystemTime},
23+
cross_process_lock::{
24+
CrossProcessLockGeneration,
25+
memory_store_helper::{Lease, try_take_leased_lock},
26+
},
27+
ring_buffer::RingBuffer,
2828
};
29+
use ruma::{MxcUri, OwnedMxcUri, time::SystemTime};
2930

3031
use super::Result;
3132
use crate::media::{
@@ -48,7 +49,7 @@ pub struct MemoryMediaStore {
4849
#[derive(Debug)]
4950
struct MemoryMediaStoreInner {
5051
media: RingBuffer<MediaContent>,
51-
leases: HashMap<String, (String, Instant)>,
52+
leases: HashMap<String, Lease>,
5253
media_retention_policy: Option<MediaRetentionPolicy>,
5354
last_media_cleanup_time: SystemTime,
5455
}
@@ -110,7 +111,7 @@ impl MediaStore for MemoryMediaStore {
110111
lease_duration_ms: u32,
111112
key: &str,
112113
holder: &str,
113-
) -> Result<bool, Self::Error> {
114+
) -> Result<Option<CrossProcessLockGeneration>, Self::Error> {
114115
let mut inner = self.inner.write().unwrap();
115116

116117
Ok(try_take_leased_lock(&mut inner.leases, lease_duration_ms, key, holder))

‎crates/matrix-sdk-base/src/media/store/mod.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ use std::fmt;
3232
use std::{ops::Deref, sync::Arc};
3333

3434
use matrix_sdk_common::cross_process_lock::{
35-
CrossProcessLock, CrossProcessLockError, CrossProcessLockGuard, TryLock,
35+
CrossProcessLock, CrossProcessLockError, CrossProcessLockGeneration, CrossProcessLockGuard,
36+
TryLock,
3637
};
3738
use matrix_sdk_store_encryption::Error as StoreEncryptionError;
3839
pub use traits::{DynMediaStore, IntoMediaStore, MediaStore, MediaStoreInner};
@@ -172,7 +173,7 @@ impl TryLock for LockableMediaStore {
172173
lease_duration_ms: u32,
173174
key: &str,
174175
holder: &str,
175-
) -> std::result::Result<bool, Self::LockError> {
176+
) -> std::result::Result<Option<CrossProcessLockGeneration>, Self::LockError> {
176177
self.0.try_take_leased_lock(lease_duration_ms, key, holder).await
177178
}
178179
}

‎crates/matrix-sdk-base/src/media/store/traits.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
use std::{fmt, sync::Arc};
1818

1919
use async_trait::async_trait;
20-
use matrix_sdk_common::AsyncTraitDeps;
20+
use matrix_sdk_common::{AsyncTraitDeps, cross_process_lock::CrossProcessLockGeneration};
2121
use ruma::{MxcUri, time::SystemTime};
2222

2323
#[cfg(doc)]
@@ -41,7 +41,7 @@ pub trait MediaStore: AsyncTraitDeps {
4141
lease_duration_ms: u32,
4242
key: &str,
4343
holder: &str,
44-
) -> Result<bool, Self::Error>;
44+
) -> Result<Option<CrossProcessLockGeneration>, Self::Error>;
4545

4646
/// Add a media file's content in the media store.
4747
///
@@ -313,7 +313,7 @@ impl<T: MediaStore> MediaStore for EraseMediaStoreError<T> {
313313
lease_duration_ms: u32,
314314
key: &str,
315315
holder: &str,
316-
) -> Result<bool, Self::Error> {
316+
) -> Result<Option<CrossProcessLockGeneration>, Self::Error> {
317317
self.0.try_take_leased_lock(lease_duration_ms, key, holder).await.map_err(Into::into)
318318
}
319319

0 commit comments

Comments
 (0)