Backport change to error handling logic in db.close().

jmhbnz · jmhbnz · commit 8165a4052203 · 2023-03-26T11:50:05.000+13:00
Signed-off-by: James Blair &lt;mail@jamesblair.net&gt;
diff --git a/db.go b/db.go
@@ -649,9 +649,10 @@ func (db *DB) close() error {
 	// Clear ops.
 	db.ops.writeAt = nil
 
+	var errs []error
 	// Close the mmap.
 	if err := db.munmap(); err != nil {
-		return err
+		errs = append(errs, err)
 	}
 
 	// Close file handles.
@@ -660,18 +661,22 @@ func (db *DB) close() error {
 		if !db.readOnly {
 			// Unlock the file.
 			if err := funlock(db); err != nil {
-				return fmt.Errorf("bolt.Close(): funlock error: %w", err)
+				errs = append(errs, fmt.Errorf("bolt.Close(): funlock error: %w", err))
 			}
 		}
 
 		// Close the file descriptor.
 		if err := db.file.Close(); err != nil {
-			return fmt.Errorf("db file close: %s", err)
+			errs = append(errs, fmt.Errorf("db file close: %w", err))
 		}
 		db.file = nil
 	}
 
 	db.path = ""
+
+	if len(errs) > 0 {
+		return errs[0]
+	}
 	return nil
 }
 
diff --git a/tests/failpoint/db_failpoint_test.go b/tests/failpoint/db_failpoint_test.go
@@ -3,6 +3,7 @@ package failpoint
 import (
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -23,3 +24,25 @@ func TestFailpoint_MapFail(t *testing.T) {
 	require.Error(t, err)
 	require.ErrorContains(t, err, "map somehow failed")
 }
+
+// ensures when munmap fails, the flock is unlocked
+func TestFailpoint_UnmapFail_DbClose(t *testing.T) {
+	//unmap error on db close
+	//we need to open the db first, and then enable the error.
+	//otherwise the db cannot be opened.
+	f := filepath.Join(t.TempDir(), "db")
+
+	err := gofail.Enable("unmapError", `return("unmap somehow failed")`)
+	require.NoError(t, err)
+	_, err = bolt.Open(f, 0666, nil)
+	require.Error(t, err)
+	require.ErrorContains(t, err, "unmap somehow failed")
+	//disable the error, and try to reopen the db
+	err = gofail.Disable("unmapError")
+	require.NoError(t, err)
+
+	db, err := bolt.Open(f, 0666, &bolt.Options{Timeout: 30 * time.Second})
+	require.NoError(t, err)
+	err = db.Close()
+	require.NoError(t, err)
+}

Original file line number	Diff line number	Diff line change
`@@ -649,9 +649,10 @@ func (db *DB) close() error {`
`649`	`649`	`// Clear ops.`
`650`	`650`	`db.ops.writeAt = nil`
`651`	`651`
	`652`	`+ var errs []error`
`652`	`653`	`// Close the mmap.`
`653`	`654`	`if err := db.munmap(); err != nil {`
`654`		`- return err`
	`655`	`+ errs = append(errs, err)`
`655`	`656`	`}`
`656`	`657`
`657`	`658`	`// Close file handles.`
`@@ -660,18 +661,22 @@ func (db *DB) close() error {`
`660`	`661`	`if !db.readOnly {`
`661`	`662`	`// Unlock the file.`
`662`	`663`	`if err := funlock(db); err != nil {`
`663`		`- return fmt.Errorf("bolt.Close(): funlock error: %w", err)`
	`664`	`+ errs = append(errs, fmt.Errorf("bolt.Close(): funlock error: %w", err))`
`664`	`665`	`}`
`665`	`666`	`}`
`666`	`667`
`667`	`668`	`// Close the file descriptor.`
`668`	`669`	`if err := db.file.Close(); err != nil {`
`669`		`- return fmt.Errorf("db file close: %s", err)`
	`670`	`+ errs = append(errs, fmt.Errorf("db file close: %w", err))`
`670`	`671`	`}`
`671`	`672`	`db.file = nil`
`672`	`673`	`}`
`673`	`674`
`674`	`675`	`db.path = ""`
	`676`	`+`
	`677`	`+ if len(errs) > 0 {`
	`678`	`+ return errs[0]`
	`679`	`+ }`
`675`	`680`	`return nil`
`676`	`681`	`}`
`677`	`682`