Skip to content

Commit 73ad518

Browse files
authored
feat: implement time lease based locks for the CryptoStore (#2140)
This implements a new time lease based lock for the `CryptoStore`, that doesn't require explicit unlocking, so that's more robust in the context of #1928, where any process may die because the device is running out of battery, or unexpected flows cause a lock to not be released properly in one or the other process. ``` //! This is a per-process lock that may be used only for very specific use //! cases, where multiple processes might concurrently write to the same //! database at the same time; this would invalidate crypto store caches, so //! that should be done mindfully. Such a lock can be acquired multiple times by //! the same process, and it remains active as long as there's at least one user //! in a given process. //! //! The lock is implemented using time-based leases to values inserted in a //! crypto store. The store maintains the lock identifier (key), who's the //! current holder (value), and an expiration timestamp on the side; see also //! `CryptoStore::try_take_leased_lock` for more details. //! //! The lock is initially acquired for a certain period of time (namely, the //! duration of a lease, aka `LEASE_DURATION_MS`), and then a "heartbeat" task //! renews the lease to extend its duration, every so often (namely, every //! `EXTEND_LEASE_EVERY_MS`). Since the tokio scheduler might be busy, the //! extension request should happen way more frequently than the duration of a //! lease, in case a deadline is missed. The current values have been chosen to //! reflect that, with a ratio of 1:10 as of 2023-06-23. //! //! Releasing the lock happens naturally, by not renewing a lease. It happens //! automatically after the duration of the last lease, at most. ``` --- * feat: implement a time lease based lock for the crypto store * feat: switch the crypto-store lock a time-leased based one * chore: fix CI, don't use unixepoch in sqlite and do time math in rust * chore: dummy implementation in indexeddb, don't run lease locks tests there * feat: in NSE, wait the duration of a lease if first attempt to unlock failed * feat: immediately release the lock when there are no more holders * chore: clippy * chore: add comment about atomic sanity * chore: increase sleeps in timeline queue tests? * feat: lower lease and renew durations * feat: keep track of the extend-lease task * fix: increment num_holders when acquiring the lock for the first time * chore: reduce indent + abort prev renew task on non-wasm + add logs
1 parent eab1f87 commit 73ad518

File tree

11 files changed

+509
-201
lines changed

11 files changed

+509
-201
lines changed

crates/matrix-sdk-crypto/src/store/integration_tests.rs

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ macro_rules! cryptostore_integration_tests {
5959
device_id!("BOBDEVICE")
6060
}
6161

62-
async fn get_loaded_store(name: &str) -> (ReadOnlyAccount, impl CryptoStore) {
62+
pub async fn get_loaded_store(name: &str) -> (ReadOnlyAccount, impl CryptoStore) {
6363
let store = get_store(name, None).await;
6464
let account = get_account();
6565
store.save_account(account.clone()).await.expect("Can't save account");
@@ -855,3 +855,76 @@ macro_rules! cryptostore_integration_tests {
855855
}
856856
};
857857
}
858+
859+
#[allow(unused_macros)]
860+
#[macro_export]
861+
macro_rules! cryptostore_integration_tests_time {
862+
() => {
863+
mod cryptostore_integration_tests_time {
864+
use std::time::Duration;
865+
866+
use matrix_sdk_test::async_test;
867+
use $crate::store::CryptoStore as _;
868+
869+
use super::cryptostore_integration_tests::*;
870+
871+
#[async_test]
872+
async fn test_lease_locks() {
873+
let (_account, store) = get_loaded_store("lease_locks").await;
874+
875+
let acquired0 = store.try_take_leased_lock(0, "key", "alice").await.unwrap();
876+
assert!(acquired0);
877+
878+
// Should extend the lease automatically (same holder).
879+
let acquired2 = store.try_take_leased_lock(300, "key", "alice").await.unwrap();
880+
assert!(acquired2);
881+
882+
// Should extend the lease automatically (same holder + time is ok).
883+
let acquired3 = store.try_take_leased_lock(300, "key", "alice").await.unwrap();
884+
assert!(acquired3);
885+
886+
// Another attempt at taking the lock should fail, because it's taken.
887+
let acquired4 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
888+
assert!(!acquired4);
889+
890+
// Even if we insist.
891+
let acquired5 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
892+
assert!(!acquired5);
893+
894+
// That's a nice test we got here, go take a little nap.
895+
tokio::time::sleep(Duration::from_millis(50)).await;
896+
897+
// Still too early.
898+
let acquired55 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
899+
assert!(!acquired55);
900+
901+
// Ok you can take another nap then.
902+
tokio::time::sleep(Duration::from_millis(250)).await;
903+
904+
// At some point, we do get the lock.
905+
let acquired6 = store.try_take_leased_lock(0, "key", "bob").await.unwrap();
906+
assert!(acquired6);
907+
908+
tokio::time::sleep(Duration::from_millis(1)).await;
909+
910+
// The other gets it almost immediately too.
911+
let acquired7 = store.try_take_leased_lock(0, "key", "alice").await.unwrap();
912+
assert!(acquired7);
913+
914+
tokio::time::sleep(Duration::from_millis(1)).await;
915+
916+
// But when we take a longer lease...
917+
let acquired8 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
918+
assert!(acquired8);
919+
920+
// It blocks the other user.
921+
let acquired9 = store.try_take_leased_lock(300, "key", "alice").await.unwrap();
922+
assert!(!acquired9);
923+
924+
// We can hold onto our lease.
925+
let acquired10 = store.try_take_leased_lock(300, "key", "bob").await.unwrap();
926+
assert!(acquired10);
927+
}
928+
}
929+
};
930+
}

0 commit comments

Comments
 (0)