Merge pull request #5882 from horizon-rust/launcher-win-account-flaky

🤖 Agent: fixes the Windows-only account test flakes that blocked launcher 0.2.33 (per-store account lock). Verified: windows-latest green twice on this head (runs 41300356019 push, 41300386770 dry run: 352 passed), Mac mini account tests + clippy clean.

launcher/src/account.rs

@@ -623,10 +623,16 @@ pub trait SecretStore: Send + Sync {
     fn lock_file(&self) -> Option<PathBuf> {
         None
     }
+    /// The in-process half of that lock. The system keychain is one per computer, so every
+    /// handle to it shares the process-wide lock; a memory store's secrets are its own, and so
+    /// is its lock: two of them (one per unit test) never wait for each other.
+    fn operations(&self) -> &Mutex<()> {
+        session::process_lock()
+    }
 }
 
 #[derive(Default)]
-pub struct MemoryStore(Mutex<HashMap<String, String>>);
+pub struct MemoryStore(Mutex<HashMap<String, String>>, Mutex<()>);
 impl SecretStore for MemoryStore {
     fn get(&self, key: &str) -> Result<Option<String>, String> {
         Ok(self.0.lock().unwrap().get(key).cloned())
@@ -642,6 +648,9 @@ impl SecretStore for MemoryStore {
     fn persistent(&self) -> bool {
         false
     }
+    fn operations(&self) -> &Mutex<()> {
+        &self.1
+    }
 }
 
 /// The Secret Service (GNOME Keyring, KWallet) through oo7, as gpui itself uses it.

launcher/src/account/session.rs

@@ -103,10 +103,14 @@ impl Status {
     }
 }
 
-static OPERATIONS: Mutex<()> = Mutex::new(());
+/// The process lock of the stores that share one keychain ([`SecretStore::operations`]).
+pub(super) fn process_lock() -> &'static Mutex<()> {
+    static OPERATIONS: Mutex<()> = Mutex::new(());
+    &OPERATIONS
+}
 
-pub(super) struct Operation {
-    _process: MutexGuard<'static, ()>,
+pub(super) struct Operation<'a> {
+    _process: MutexGuard<'a, ()>,
     _file: Option<File>,
 }
 
@@ -117,7 +121,7 @@ const BUSY: &str = "Another account operation is running. Try again in a moment.
 
 /// Serialize refresh with sign-in, sign-out and launch, before reading any credential.
 /// The file is a lock only: it never contains tokens. Fail closed if locking fails.
-pub(super) fn lock(store: &dyn SecretStore) -> Result<Operation, String> {
+pub(super) fn lock(store: &dyn SecretStore) -> Result<Operation<'_>, String> {
     lock_within(store, None)
 }
 
@@ -125,7 +129,7 @@ pub(super) fn lock(store: &dyn SecretStore) -> Result<Operation, String> {
 pub(super) fn lock_within(
     store: &dyn SecretStore,
     within: Option<Duration>,
-) -> Result<Operation, String> {
+) -> Result<Operation<'_>, String> {
     let deadline = within.map(|d| Instant::now() + d);
     // Waits for `take` to stop answering None (busy), or until the deadline.
     let wait = |take: &mut dyn FnMut() -> Result<Option<()>, String>| loop {
@@ -138,8 +142,8 @@ pub(super) fn lock_within(
     let failed = "Account operation failed. Restart the launcher.";
     let mut process = None;
     match deadline {
-        None => process = Some(OPERATIONS.lock().map_err(|_| failed)?),
-        Some(_) => wait(&mut || match OPERATIONS.try_lock() {
+        None => process = Some(store.operations().lock().map_err(|_| failed)?),
+        Some(_) => wait(&mut || match store.operations().try_lock() {
             Ok(guard) => {
                 process = Some(guard);
                 Ok(Some(()))

launcher/src/account/tests.rs

@@ -421,6 +421,9 @@ impl SecretStore for Refusing {
     fn persistent(&self) -> bool {
         true
     }
+    fn operations(&self) -> &Mutex<()> {
+        self.0.operations()
+    }
 }
 
 #[test]
@@ -861,6 +864,9 @@ impl SecretStore for Reading<'_> {
     fn persistent(&self) -> bool {
         false
     }
+    fn operations(&self) -> &Mutex<()> {
+        self.0.operations()
+    }
 }
 
 #[test]
@@ -1455,6 +1461,9 @@ fn a_status_read_gives_up_behind_a_long_operation() {
     let busy = session::lock_within(&store, Some(std::time::Duration::from_millis(150)));
     assert!(busy.is_err(), "a held lock must not be taken twice");
     assert!(start.elapsed() < std::time::Duration::from_secs(2));
+    // Another memory store (another test running beside this one) has its own lock.
+    let other = MemoryStore::default();
+    assert!(session::lock_within(&other, Some(std::time::Duration::ZERO)).is_ok());
     drop(held);
     assert!(session::lock_within(&store, Some(std::time::Duration::from_secs(5))).is_ok());
 }