Browse Source

fix(database): close the pool a second InitDB replaces

InitDB assigned the new pool over the old one without closing it. The
panel's own restore flows call CloseDB first, but any other re-init
leaked the replaced pool and its handle on the database file. On Windows
that handle blocks deleting the file, which is why the four GetApiToken
CLI tests failed their t.TempDir cleanup there: dbtest.InitDB opened the
store, then GetApiToken's own InitDB replaced it. InitDB now closes the
previous pool itself; sql.DB.Close is idempotent, so the restore flows
behave as before.
MHSanaei 7 hours ago
parent
commit
18b337d131
2 changed files with 32 additions and 0 deletions
  1. 5 0
      internal/database/db.go
  2. 27 0
      internal/database/db_reopen_test.go

+ 5 - 0
internal/database/db.go

@@ -2698,6 +2698,11 @@ func InitDB(dbPath string) error {
 	}
 	c := &gorm.Config{Logger: gormLogger, DisableForeignKeyConstraintWhenMigrating: true}
 
+	// Reopening replaces the process pool; the replaced one would keep its file open.
+	if err := CloseDB(); err != nil {
+		log.Printf("close the replaced database pool: %v", err)
+	}
+
 	var err error
 	switch config.GetDBKind() {
 	case "postgres":

+ 27 - 0
internal/database/db_reopen_test.go

@@ -0,0 +1,27 @@
+package database
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+// A replaced pool that stays open keeps its database file open; Windows then
+// cannot delete or replace that file.
+func TestInitDBClosesThePoolItReplaces(t *testing.T) {
+	dbPath := filepath.Join(t.TempDir(), "x-ui.db")
+	if err := InitDB(dbPath); err != nil {
+		t.Fatalf("first InitDB: %v", err)
+	}
+	t.Cleanup(func() { _ = CloseDB() })
+	replaced, err := GetDB().DB()
+	if err != nil {
+		t.Fatalf("first pool: %v", err)
+	}
+
+	if err := InitDB(dbPath); err != nil {
+		t.Fatalf("second InitDB: %v", err)
+	}
+	if err := replaced.Ping(); err == nil || err.Error() != "sql: database is closed" {
+		t.Fatalf("replaced pool Ping() = %v, want sql: database is closed", err)
+	}
+}