From 5e04426888a721a108aba41ff95b00a3d75e74a3 Mon Sep 17 00:00:00 2001
From: JT Archie <jtarchie@gmail.com>
Date: Tue, 1 Sep 2026 19:41:54 -0600
Subject: [PATCH] do not set pMethods when xOpen fails

sqlite calls xClose on any file whose xOpen failed, as long as pMethods
is set. s3vfsOpen set it unconditionally -- but a failed goVFSOpen never
registered the file, so the s3vfsFile still carried its zero value, and
the xClose that followed looked up id 0: the first file the process ever
opened. That file was closed out from under the connection that owns it.

The owning connection then failed every read that missed its page cache,
while everything already cached kept answering. The errors said "SQL
logic error" and "fts5: corruption found reading blob ..." -- nothing
pointed at a close, a failed open of an unrelated file, or this glue.
On a long-lived process serving a large read-only database, that is a
persistent outage with a misleading name: observed in production on
2026-09-01, where one bad open broke the serving connection for hours
and the database was blamed for corruption it did not have.

Leave pMethods NULL on failure, which is sqlite's documented way of
saying there is nothing to close. Start file ids at 1 as well, so a
zeroed s3vfsFile can never alias a live file through any other path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WMBg7JBvZJRRu1RytUj5c3
---
 failedopen_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++++++
 sqlite3vfs.c       |  7 ++++
 sqlite3vfscgo.go   |  7 ++--
 3 files changed, 90 insertions(+), 3 deletions(-)
 create mode 100644 failedopen_test.go

diff --git a/failedopen_test.go b/failedopen_test.go
new file mode 100644
index 0000000..93a3729
--- /dev/null
+++ b/failedopen_test.go
@@ -0,0 +1,79 @@
+package sqlite3vfs
+
+import (
+	"database/sql"
+	"fmt"
+	"testing"
+
+	_ "github.com/mattn/go-sqlite3"
+)
+
+// recordingVFS remembers every file it opens.
+type recordingVFS struct {
+	*TmpVFS
+	opened []*TmpFile
+}
+
+func (vfs *recordingVFS) Open(name string, flags OpenFlag) (File, OpenFlag, error) {
+	f, retFlags, err := vfs.TmpVFS.Open(name, flags)
+	if err != nil {
+		return nil, 0, err
+	}
+
+	tf := f.(*TmpFile)
+	vfs.opened = append(vfs.opened, tf)
+	return tf, retFlags, nil
+}
+
+// TestFailedOpenDoesNotCloseAnotherFile opens one database successfully, then
+// fails to open a second, and checks the first database's file is still open.
+//
+// sqlite calls xClose on a file whose xOpen failed whenever pMethods is set.
+// s3vfsOpen set pMethods unconditionally, but a failed goVFSOpen never stamped
+// an id into the (zeroed) s3vfsFile — so that xClose arrived with id 0 and
+// closed fileMap[0]: the first file this process ever opened, out from under
+// the connection that owns it. That connection then failed every read that
+// missed its page cache, with errors ("SQL logic error", fts5 corruption
+// reports) that pointed anywhere but here.
+func TestFailedOpenDoesNotCloseAnotherFile(t *testing.T) {
+	vfs := &recordingVFS{TmpVFS: newTempVFS()}
+
+	vfsName := "recordingvfs"
+	if err := RegisterVFS(vfsName, vfs); err != nil {
+		t.Fatal(err)
+	}
+
+	db, err := sql.Open("sqlite3", fmt.Sprintf("first.db?vfs=%s", vfsName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer db.Close()
+
+	if _, err := db.Exec(`CREATE TABLE t (id INTEGER PRIMARY KEY)`); err != nil {
+		t.Fatal(err)
+	}
+
+	if len(vfs.opened) == 0 {
+		t.Fatal("no file was opened for first.db")
+	}
+	first := vfs.opened[0]
+
+	// mode=ro of a missing file: TmpVFS refuses the open.
+	missing, err := sql.Open("sqlite3", fmt.Sprintf("file:does-not-exist.db?mode=ro&vfs=%s", vfsName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := missing.Ping(); err == nil {
+		t.Fatal("opening a missing database in mode=ro should fail")
+	}
+	missing.Close()
+
+	if _, err := first.f.Stat(); err != nil {
+		t.Fatalf("first.db's file was closed by the failed open of an unrelated file: %v", err)
+	}
+
+	var count int
+	if err := db.QueryRow(`SELECT COUNT(*) FROM t`).Scan(&count); err != nil {
+		t.Fatalf("first.db is broken after the failed open of an unrelated file: %v", err)
+	}
+}
diff --git a/sqlite3vfs.c b/sqlite3vfs.c
index 6abd1eb..e96e91f 100644
--- a/sqlite3vfs.c
+++ b/sqlite3vfs.c
@@ -61,6 +61,13 @@ int s3vfsNew(char* name, int maxPathName) {
 
 int s3vfsOpen(sqlite3_vfs* vfs, const char * name, sqlite3_file* file, int flags, int *outFlags) {
   int ret = goVFSOpen(vfs, name, file, flags, outFlags);
+  /* sqlite calls xClose on a failed open whenever pMethods is set, and a
+  ** failed goVFSOpen never registered this file -- its zeroed id would alias
+  ** another live file. NULL means there is nothing to close. */
+  if (ret != SQLITE_OK) {
+    file->pMethods = 0;
+    return ret;
+  }
   file->pMethods = &s3vfs_io_methods;
   return ret;
 }
diff --git a/sqlite3vfscgo.go b/sqlite3vfscgo.go
index 0f5f7b9..208482f 100644
--- a/sqlite3vfscgo.go
+++ b/sqlite3vfscgo.go
@@ -17,9 +17,10 @@ import (
 var (
 	vfsMap = make(map[string]ExtendedVFSv1)
 
-	fileMux    sync.Mutex
-	nextFileID uint64
-	fileMap    = make(map[uint64]File)
+	fileMux sync.Mutex
+	// Starts at 1 so a zeroed s3vfsFile's id can never name a live file.
+	nextFileID uint64 = 1
+	fileMap           = make(map[uint64]File)
 )
 
 func newVFS(name string, goVFS ExtendedVFSv1, maxPathName int) error {