Merge pull request #439 from jmhbnz/backport-failpoints-injection

[1.3] Backport change to error handling logic

Author: ahrtr

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
 }
 

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)
+}