fix snapshot deadlock and cleanup mutex in createSession (#780)

* fix snapshot deadlock and cleanup mutex in createSession

* remove faulty optimisatino

* review

* return treeInfo if no atom

* with dep update not needed

---------

Co-authored-by: Christophe Diederichs <chm-diederichs@protonmail.com>

Author: mafintosh

index.js

@@ -376,16 +376,12 @@ class Hypercore extends EventEmitter {
     const state = this.state
 
     if (opts.atom) {
-      await parentState.mutex.lock()
       this.state = await parentState.createSession(null, false, opts.atom)
-      parentState.mutex.unlock()
       if (state) state.unref()
     } else if (opts.name) {
       // todo: need to make named sessions safe before ready
       // atm we always copy the state in passCapabilities
-      await parentState.mutex.lock()
       this.state = await parentState.createSession(opts.name, !!opts.overwrite, null)
-      parentState.mutex.unlock()
       if (state) state.unref() // ref'ed above in setup session
     }
 

lib/session-state.js

@@ -124,8 +124,7 @@ class SessionState {
   }
 
   async _updateSnapshotStorage(storage) {
-    if (!this.atomized || !this.atomized.flushing) return this.treeInfo()
-    await this.atomized.flushed()
+    while (this.atomized && this.atomized.flushing) await this.atomized.flushed()
 
     let rx = storage.read()
     const headPromise = rx.getHead()
@@ -206,16 +205,10 @@ class SessionState {
   }
 
   async snapshot() {
-    try {
-      await this.mutex.lock()
-
-      const storage = this.storage.snapshot()
-      const treeInfo = await this._updateSnapshotStorage(storage)
+    const storage = this.storage.snapshot()
+    const treeInfo = await this._updateSnapshotStorage(storage)
 
-      return new SessionState(this.core, null, storage, treeInfo, this.name)
-    } finally {
-      this._unlock()
-    }
+    return new SessionState(this.core, null, storage, treeInfo, this.name)
   }
 
   updateDependency(tx, length) {
@@ -1137,56 +1130,62 @@ class SessionState {
   }
 
   async createSession(name, overwrite, atom) {
-    let storage = null
-    let treeInfo = null
+    await this.mutex.lock()
 
-    if (!atom && !overwrite && this.storage) {
-      storage = await this.storage.resumeSession(name)
+    try {
+      let storage = null
+      let treeInfo = null
 
-      if (storage !== null) treeInfo = (await getCoreHead(storage)) || getDefaultTree()
-    }
+      if (!atom && !overwrite && this.storage) {
+        storage = await this.storage.resumeSession(name)
 
-    const length = treeInfo ? treeInfo.length : this.length
+        if (storage !== null) treeInfo = (await getCoreHead(storage)) || getDefaultTree()
+      }
 
-    if (storage === null) {
-      treeInfo = await this._getTreeHeadAt(length)
+      const length = treeInfo ? treeInfo.length : this.length
 
-      if (atom) {
-        storage = await this.storage.createAtomicSession(atom, treeInfo)
-      } else {
-        storage = await this.storage.createSession(name, treeInfo)
+      if (storage === null) {
+        treeInfo = await this._getTreeHeadAt(length)
+
+        if (atom) {
+          storage = await this.storage.createAtomicSession(atom, treeInfo)
+        } else {
+          storage = await this.storage.createSession(name, treeInfo)
+        }
       }
-    }
 
-    if (this.atomized && atom) {
-      throw new Error('Session already atomized')
-    }
+      if (this.atomized && atom) {
+        throw new Error('Session already atomized')
+      }
 
-    const head = {
-      fork: this.fork,
-      roots:
-        length === this.length
-          ? this.roots.slice()
-          : await MerkleTree.getRootsFromStorage(storage, length),
-      length,
-      prologue: this.prologue,
-      signature: length === this.length ? this.signature : null
-    }
+      const head = {
+        fork: this.fork,
+        roots:
+          length === this.length
+            ? this.roots.slice()
+            : await MerkleTree.getRootsFromStorage(storage, length),
+        length,
+        prologue: this.prologue,
+        signature: length === this.length ? this.signature : null
+      }
 
-    const state = new SessionState(
-      this.core,
-      atom ? this : null,
-      storage,
-      head,
-      atom ? this.name : name
-    )
+      const state = new SessionState(
+        this.core,
+        atom ? this : null,
+        storage,
+        head,
+        atom ? this.name : name
+      )
 
-    if (atom) {
-      this.atomized = atom
-      atom.onflush(state._commit.bind(state))
-    }
+      if (atom) {
+        this.atomized = atom
+        atom.onflush(state._commit.bind(state))
+      }
 
-    return state
+      return state
+    } finally {
+      this._unlock()
+    }
   }
 }