From 3810cbb71f032807fa876d48646f87c70cb1fb4b Mon Sep 17 00:00:00 2001
From: Brian Botha <brian.botha@matrix.ai>
Date: Thu, 11 Aug 2022 14:30:25 +1000
Subject: [PATCH] fix: vaults locking and transactional locking

---
 src/vaults/VaultInternal.ts | 27 ++++++++++++++-------------
 src/vaults/VaultManager.ts  | 14 ++++++++------
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/src/vaults/VaultInternal.ts b/src/vaults/VaultInternal.ts
index cccaabad66..0b5d95e7f4 100644
--- a/src/vaults/VaultInternal.ts
+++ b/src/vaults/VaultInternal.ts
@@ -445,23 +445,24 @@ class VaultInternal {
       return this.db.withTransactionF((tran) => this.writeF(f, tran));
     }
 
-    // This should really be an internal property
-    // get whether this is remote, and the remote address
-    // if it is, we consider this repo an "attached repo"
-    // this vault is a "mirrored" vault
-    if (
-      (await tran.get([
-        ...this.vaultMetadataDbPath,
-        VaultInternal.remoteKey,
-      ])) != null
-    ) {
-      // Mirrored vaults are immutable
-      throw new vaultsErrors.ErrorVaultRemoteDefined();
-    }
     return withF([this.lock.write()], async () => {
       await tran.lock(
         [...this.vaultMetadataDbPath, VaultInternal.dirtyKey].toString(),
       );
+
+      // This should really be an internal property
+      // get whether this is remote, and the remote address
+      // if it is, we consider this repo an "attached repo"
+      // this vault is a "mirrored" vault
+      if (
+        (await tran.get([
+          ...this.vaultMetadataDbPath,
+          VaultInternal.remoteKey,
+        ])) != null
+      ) {
+        // Mirrored vaults are immutable
+        throw new vaultsErrors.ErrorVaultRemoteDefined();
+      }
       await tran.put(
         [...this.vaultMetadataDbPath, VaultInternal.dirtyKey],
         true,
diff --git a/src/vaults/VaultManager.ts b/src/vaults/VaultManager.ts
index ba3339270a..e26b593b2c 100644
--- a/src/vaults/VaultManager.ts
+++ b/src/vaults/VaultManager.ts
@@ -365,12 +365,15 @@ class VaultManager {
       );
     }
 
-    const vaultMeta = await this.getVaultMeta(vaultId, tran);
-    if (vaultMeta == null) return;
-    const vaultName = vaultMeta.vaultName;
-    this.logger.info(`Destroying Vault ${vaultsUtils.encodeVaultId(vaultId)}`);
-    const vaultIdString = vaultId.toString() as VaultIdString;
     await this.vaultLocks.withF([vaultId, RWLockWriter, 'write'], async () => {
+      const vaultMeta = await this.getVaultMeta(vaultId, tran);
+      if (vaultMeta == null) return;
+      const vaultName = vaultMeta.vaultName;
+      await tran.lock([...this.vaultsNamesDbPath, vaultName].toString());
+      this.logger.info(
+        `Destroying Vault ${vaultsUtils.encodeVaultId(vaultId)}`,
+      );
+      const vaultIdString = vaultId.toString() as VaultIdString;
       const vault = await this.getVault(vaultId, tran);
       // Destroying vault state and metadata
       await vault.stop();
@@ -378,7 +381,6 @@ class VaultManager {
       // Removing from map
       this.vaultMap.delete(vaultIdString);
       // Removing name->id mapping
-      await tran.lock([...this.vaultsNamesDbPath, vaultName].toString());
       await tran.del([...this.vaultsNamesDbPath, vaultName]);
     });
     this.logger.info(`Destroyed Vault ${vaultsUtils.encodeVaultId(vaultId)}`);