From 1158810fe48d3595df83bc2110c0831bc122c189 Mon Sep 17 00:00:00 2001
From: "kayos@tcp.direct" <kayos@tcp.direct>
Date: Wed, 3 Jul 2024 16:21:08 -0700
Subject: [PATCH] Fix: deadlock on sync all

---
 bitcask/bitcask.go | 20 +++++++++++++-------
 go.mod             |  1 +
 pogreb/pogreb.go   | 33 ++++++++++++++++++++++-----------
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/bitcask/bitcask.go b/bitcask/bitcask.go
index ddd1952..fbc9843 100644
--- a/bitcask/bitcask.go
+++ b/bitcask/bitcask.go
@@ -71,18 +71,23 @@ func (db *DB) Meta() models.Metadata {
 	return m
 }
 
+func (db *DB) allStores() map[string]database.Filer {
+	var stores = make(map[string]database.Filer)
+	for n, s := range db.store {
+		stores[n] = s
+	}
+	return stores
+}
+
 // AllStores returns a map of the names of all bitcask datastores and the corresponding Filers.
 func (db *DB) AllStores() map[string]database.Filer {
 	if err := db.init(); err != nil {
 		panic(err)
 	}
 	db.mu.RLock()
-	defer db.mu.RUnlock()
-	var stores = make(map[string]database.Filer)
-	for n, s := range db.store {
-		stores[n] = s
-	}
-	return stores
+	ast := db.allStores()
+	db.mu.RUnlock()
+	return ast
 }
 
 func (db *DB) _init() error {
@@ -504,7 +509,7 @@ func (db *DB) CloseAll() error {
 	return err
 }
 func (db *DB) addAllStoresToMeta() {
-	storeMap := db.AllStores()
+	storeMap := db.allStores()
 	storeNames := make([]string, len(storeMap))
 	for name := range storeMap {
 		storeNames = append(storeNames, name)
@@ -513,6 +518,7 @@ func (db *DB) addAllStoresToMeta() {
 }
 
 // SyncAll syncs all pogreb datastores.
+// TODO: investigate locking here, right now if we try to hold a lock during a backup we'll hang :^)
 func (db *DB) SyncAll() error {
 	db.addAllStoresToMeta()
 	var errs = make([]error, 0)
diff --git a/go.mod b/go.mod
index 3ed2500..10c7216 100644
--- a/go.mod
+++ b/go.mod
@@ -23,6 +23,7 @@ require (
 )
 
 retract (
+	v0.4.3 // deadlock
 	v0.4.0 // broken metadata system
 	v0.3.0 // doesn't pass go vet
 )
diff --git a/pogreb/pogreb.go b/pogreb/pogreb.go
index 2141f40..bf45ba5 100644
--- a/pogreb/pogreb.go
+++ b/pogreb/pogreb.go
@@ -175,18 +175,21 @@ func (db *DB) Meta() models.Metadata {
 	return m
 }
 
-func (db *DB) UpdateMetrics() {
-	db.mu.RLock()
-	defer db.mu.RUnlock()
+func (db *DB) updateMetrics() {
 	for _, s := range db.store {
-		s.metrics = s.DB.Metrics()
+		if s != nil && s.DB != nil {
+			s.metrics = s.DB.Metrics()
+		}
 	}
 }
 
-// AllStores returns a map of the names of all pogreb datastores and the corresponding Filers.
-func (db *DB) AllStores() map[string]database.Filer {
+func (db *DB) UpdateMetrics() {
 	db.mu.RLock()
-	defer db.mu.RUnlock()
+	db.updateMetrics()
+	db.mu.RUnlock()
+}
+
+func (db *DB) allStores() map[string]database.Filer {
 	var stores = make(map[string]database.Filer)
 	for n, s := range db.store {
 		stores[n] = s
@@ -194,6 +197,15 @@ func (db *DB) AllStores() map[string]database.Filer {
 	return stores
 }
 
+// AllStores returns a map of the names of all pogreb datastores and the corresponding Filers.
+func (db *DB) AllStores() map[string]database.Filer {
+
+	db.mu.RLock()
+	ast := db.allStores()
+	db.mu.RUnlock()
+	return ast
+}
+
 // FIXME: not returning the error is probably pretty irresponsible.
 
 // OpenDB will either open an existing set of pogreb datastores at the given directory, or it will create a new one.
@@ -522,21 +534,19 @@ func (db *DB) CloseAll() error {
 }
 
 func (db *DB) allMetrics() map[string]*pogreb.Metrics {
-	db.UpdateMetrics()
+	db.updateMetrics()
 	allmet := make(map[string]*pogreb.Metrics, len(db.store))
-	db.mu.RLock()
 	for name, store := range db.store {
 		if store == nil || store.Backend().(*pogreb.DB) == nil {
 			continue
 		}
 		allmet[name] = store.metrics
 	}
-	db.mu.RUnlock()
 	return allmet
 }
 
 func (db *DB) addAllStoresToMeta() {
-	storeMap := db.AllStores()
+	storeMap := db.allStores()
 	storeNames := make([]string, len(storeMap))
 	for name := range storeMap {
 		storeNames = append(storeNames, name)
@@ -550,6 +560,7 @@ func (db *DB) syncMetaValues() {
 }
 
 // SyncAll syncs all pogreb datastores.
+// TODO: investigate locking here, right now if we try to hold a lock during a backup we'll hang :^)
 func (db *DB) SyncAll() error {
 	db.syncMetaValues()
 	var errs = make([]error, 0)