malice

public

Star0

Fork 0

Watch0

83 commits 2 branches 0 tags 0 releases4.4 MiB

Commit `698e2238368e`

tarball

atagen <boss@atagen.co>2026-10-25 10:50 UTC

1 file changed+69-10

server: lock sessions per sid instead of globally

Every restore, login and logout took one server-wide lock across homeserver round trips, so a slow or unreachable homeserver for one user stalled sign-in and session restore for everyone. Only the same sid needs exclusion, since that is what guards a SQLite store from being opened by two engines.

Modified file: crates/malice-server/src/state.rs+69 additions−10 deletions@@ -1,7 +1,7 @@
 use std::{
     collections::HashMap,
     path::{Path, PathBuf},
-    sync::Arc,
+    sync::{Arc, PoisonError, Weak},
     time::Duration,
 };
 
@@ -13,7 +13,11 @@ use macltok::{
 };
 use rand_core::{OsRng, RngCore};
 use sha2::{Digest, Sha256};
-use tokio::{sync::Mutex, task::JoinHandle, time::timeout};
+use tokio::{
+    sync::{Mutex, OwnedMutexGuard},
+    task::JoinHandle,
+    time::timeout,
+};
 
 use malice_core::{
     error::{CoreError, Failure},
@@ -36,9 +40,10 @@ pub struct UserSession {
 
 struct Inner {
     sessions: tokio::sync::RwLock<HashMap<String, Arc<UserSession>>>,
-    /// Serialises concurrent restores of the same sid so two engines never
-    /// open the same SQLite store at once.
-    init_lock: Mutex<()>,
+    /// Serialises restores, logins and removals of the same sid so two engines
+    /// never open the same SQLite store at once, without one slow homeserver
+    /// holding up every other user.
+    sid_locks: std::sync::Mutex<HashMap<String, Weak<Mutex<()>>>>,
     data_dir: PathBuf,
     master_key: [u8; 32],
     cookie_secure: bool,
@@ -72,7 +77,7 @@ impl AppState {
         Self {
             inner: Arc::new(Inner {
                 sessions: tokio::sync::RwLock::new(HashMap::new()),
-                init_lock: Mutex::new(()),
+                sid_locks: std::sync::Mutex::new(HashMap::new()),
                 data_dir,
                 master_key,
                 cookie_secure,
@@ -89,6 +94,25 @@ impl AppState {
         self.inner.cookie_secure
     }
 
+    async fn lock_sid(&self, sid: &str) -> OwnedMutexGuard<()> {
+        let lock = {
+            let mut locks = self
+                .inner
+                .sid_locks
+                .lock()
+                .unwrap_or_else(PoisonError::into_inner);
+            locks.retain(|_, lock| lock.strong_count() > 0);
+            if let Some(lock) = locks.get(sid).and_then(Weak::upgrade) {
+                lock
+            } else {
+                let lock = Arc::new(Mutex::new(()));
+                locks.insert(sid.to_owned(), Arc::downgrade(&lock));
+                lock
+            }
+        };
+        lock.lock_owned().await
+    }
+
     fn sessions_dir(&self) -> PathBuf {
         self.inner.data_dir.join("sessions")
     }
@@ -144,7 +168,7 @@ impl AppState {
         if let Some(user) = self.inner.sessions.read().await.get(sid).cloned() {
             return Ok(user);
         }
-        let _init = self.inner.init_lock.lock().await;
+        let _init = self.lock_sid(sid).await;
         if let Some(user) = self.inner.sessions.read().await.get(sid).cloned() {
             return Ok(user);
         }
@@ -200,7 +224,7 @@ impl AppState {
                     sessions.clone(),
                 )
                 .await?;
-                let _init = self.inner.init_lock.lock().await;
+                let _init = self.lock_sid(sid).await;
                 return self
                     .start_user_session(sid.to_owned(), session, sessions)
                     .await
@@ -212,7 +236,7 @@ impl AppState {
         let sid = new_sid();
         let sessions = self.session_store(&sid);
         let session = login_with(login, &password, sessions.clone()).await?;
-        let _init = self.inner.init_lock.lock().await;
+        let _init = self.lock_sid(&sid).await;
         self.start_user_session(sid, session, sessions)
             .await
             .map_err(|error| CoreError::new(Failure::Upstream, error))
@@ -282,7 +306,7 @@ impl AppState {
     pub async fn remove_session(&self, sid: &str, retain: Retain) {
         // A restore must not reopen the store between removing the map entry
         // and deleting its persisted credentials.
-        let _init = self.inner.init_lock.lock().await;
+        let _init = self.lock_sid(sid).await;
         let user = self.inner.sessions.write().await.remove(sid);
         let sessions = match &user {
             Some(user) => user.sessions.clone(),
@@ -533,3 +557,38 @@ fn restrict_file(path: &Path) {
 
 #[cfg(not(unix))]
 fn restrict_file(_path: &Path) {}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn state() -> (AppState, tempfile::TempDir) {
+        let dir = tempfile::tempdir().expect("temp dir");
+        (
+            AppState::new(dir.path().to_owned(), [7; 32], false, false),
+            dir,
+        )
+    }
+
+    #[tokio::test]
+    async fn sid_locks_only_serialise_the_same_sid() {
+        let (state, _dir) = state();
+        let held = state.lock_sid("alpha").await;
+        let other = timeout(Duration::from_millis(50), state.lock_sid("beta")).await;
+        assert!(other.is_ok(), "a different sid must not wait");
+        let same = timeout(Duration::from_millis(50), state.lock_sid("alpha")).await;
+        assert!(same.is_err(), "the same sid must wait for the holder");
+        drop(held);
+        let again = timeout(Duration::from_millis(50), state.lock_sid("alpha")).await;
+        assert!(again.is_ok(), "the sid is free once the holder drops");
+    }
+
+    #[tokio::test]
+    async fn sid_locks_forget_released_sids() {
+        let (state, _dir) = state();
+        drop(state.lock_sid("alpha").await);
+        drop(state.lock_sid("beta").await);
+        let live = state.inner.sid_locks.lock().unwrap().len();
+        assert!(live <= 1, "released locks are pruned on the next lookup");
+    }
+}

malice

Commit 698e2238368e

Commit `698e2238368e`