From 802bc50cd14154d4a9e3d7e7f495823435161737 Mon Sep 17 00:00:00 2001 From: franchb Date: Tue, 7 Jul 2026 01:46:00 -0500 Subject: [PATCH 1/2] vfs: fix Windows WAL corruption by mapping the wal-index into wasm memory Fixes #404. On Windows the WAL-index was kept in a per-connection private copy and synchronized with the shared -shm file only at xShmLock boundaries. But wal.c reads and writes the WAL-index *between* lock calls and relies on seeing other connections' updates live: dirty-header re-reads, aReadMark probes when a checkpointer's exclusive slot probe loses to a live reader, and post-lock header re-checks. The copy-on-lock-boundary scheme served stale views on those paths, so under concurrent WAL writes a checkpointer could backfill stale page versions into the database and truncate frames that were never backfilled, corrupting the file (and producing SQLITE_PROTOCOL error storms). On Windows 10 1803+ / Server 2019+, map the -shm regions directly into the wasm linear memory using address-space placeholders (VirtualAlloc2 + MapViewOfFile3), the analogue of the MAP_FIXED mapping the unix build already uses. SQLite then works on genuinely shared, always-current memory; xShmBarrier becomes a plain fence and the private/shadow copies and their sync points disappear. The heap is committed at 64K granularity so a wal-index view (64K aligned, 64K) never straddles two allocations, which a placeholder split cannot cross. Below Windows 10 1803 the placeholder APIs are unavailable. Rather than keep a copy-on-lock-boundary fallback that cannot make wal.c's between-lock reads coherent, shmMap refuses shared memory there (_IOERR_SHMMAP): such a build must use WAL with EXCLUSIVE locking mode, like platforms without shared-memory support. This drops the old Windows copy path entirely and the now-unused mmap_windows.go. The copy scheme (shm_copy.go) is retained for the sqlite3_dotlk build, which has no file mapping and genuinely needs it; it keeps the per-file copy serialization and exact per-page comparison that make it correct within a single process. --- internal/sqlite3_wrap/mem_windows.go | 169 +++++++++++++++++-- internal/sqlite3_wrap/mmap_windows.go | 52 ------ internal/sqlite3_wrap/placeholder_windows.go | 93 ++++++++++ vfs/shm_copy.go | 132 +++++++++++---- vfs/shm_dotlk.go | 21 +++ vfs/shm_windows.go | 159 ++++++++++------- 6 files changed, 460 insertions(+), 166 deletions(-) delete mode 100644 internal/sqlite3_wrap/mmap_windows.go create mode 100644 internal/sqlite3_wrap/placeholder_windows.go diff --git a/internal/sqlite3_wrap/mem_windows.go b/internal/sqlite3_wrap/mem_windows.go index b048d552..c73fca0b 100644 --- a/internal/sqlite3_wrap/mem_windows.go +++ b/internal/sqlite3_wrap/mem_windows.go @@ -2,16 +2,19 @@ package sqlite3_wrap import ( "math" + "os" "unsafe" "golang.org/x/sys/windows" ) type Memory struct { - Buf []byte - Max int64 - com int - ptr uintptr + Buf []byte + Max int64 + com int + ptr uintptr + placeh bool // reserved with placeholders (Win10 1803+) + views []uintptr // offsets of live file views, for Close } func (m *Memory) Slice() *[]byte { @@ -44,18 +47,30 @@ func (m *Memory) allocate(max uint64) { if res > math.MaxInt { // This ensures uintptr(res) overflows to a large value, - // and windows.VirtualAlloc returns an error. + // and the reservation fails. res = math.MaxUint64 } // Reserve res bytes of address space, to ensure we won't need to move it. - r, err := windows.VirtualAlloc(0, uintptr(res), windows.MEM_RESERVE, windows.PAGE_READWRITE) - if err != nil { - panic(err) + // Prefer a placeholder reservation (Windows 10 1803+): it can later have + // file views mapped into it, which the WAL-index shared memory uses. + m.placeh = PlaceholdersSupported() + if m.placeh { + r, err := virtualAlloc2(0, uintptr(res), + _MEM_RESERVE|_MEM_RESERVE_PLACEHOLDERS, _PAGE_NOACCESS) + if err != nil { + panic(err) + } + m.ptr = r + } else { + r, err := windows.VirtualAlloc(0, uintptr(res), windows.MEM_RESERVE, windows.PAGE_READWRITE) + if err != nil { + panic(err) + } + m.ptr = r } - m.ptr = r - ptr := *(*unsafe.Pointer)(unsafe.Pointer(&r)) + ptr := *(*unsafe.Pointer)(unsafe.Pointer(&m.ptr)) m.Buf = unsafe.Slice((*byte)(ptr), res)[:0] } @@ -69,20 +84,144 @@ func (m *Memory) reallocate(size uint64) { new = min(max(size, new), res) new = (new + rnd) &^ rnd - // Commit additional memory up to new bytes. - _, err := windows.VirtualAlloc(m.ptr, uintptr(new), windows.MEM_COMMIT, windows.PAGE_READWRITE) - if err != nil { - panic(err) + if m.placeh { + // Round to the 64K allocation granularity so every committed + // chunk boundary is 64K-aligned. A wal-index view (64K-aligned, + // 64K) can then never straddle two allocations, which a + // placeholder split cannot cross. + const gran = 64 * 1024 + new = (new + gran - 1) &^ (gran - 1) + new = min(new, res) + // Split the trailing placeholder to [com, new) and replace it + // with committed memory. Placeholder-lineage allocations can be + // carved back into placeholders later, which file-view mapping + // relies on. + if new < res { + if err := splitPlaceholder(m.ptr+uintptr(com), uintptr(new-com)); err != nil { + panic(err) + } + } + if _, err := virtualAlloc2(m.ptr+uintptr(com), uintptr(new-com), + _MEM_RESERVE|_MEM_COMMIT|_MEM_REPLACE_PLACEHOLDER, _PAGE_READWRITE); err != nil { + panic(err) + } + } else { + // Commit additional memory up to new bytes. + if _, err := windows.VirtualAlloc(m.ptr, uintptr(new), windows.MEM_COMMIT, windows.PAGE_READWRITE); err != nil { + panic(err) + } } m.com = int(new) } m.Buf = m.Buf[:size] } +// CanMapFiles reports whether file views can be mapped into this memory. +func (m *Memory) CanMapFiles() bool { return m.placeh } + +// MapFileRegion maps size bytes of f at offset fileOff into the linear +// memory at offset addrOff, replacing previously committed private memory. +// addrOff must be allocation-granularity aligned and [addrOff, addrOff+size) +// must lie within a single committed chunk (e.g. one sqlite3_malloc block +// well inside the heap). The caller owns making the range unused by Go/wasm +// code for the lifetime of the view. +func (m *Memory) MapFileRegion(f *os.File, fileOff int64, addrOff, size uintptr) error { + addr := m.ptr + addrOff + if err := splitPlaceholder(addr, size); err != nil { + return err + } + maxSize := uint64(fileOff) + uint64(size) + h, err := windows.CreateFileMapping(windows.Handle(f.Fd()), nil, + windows.PAGE_READWRITE, uint32(maxSize>>32), uint32(maxSize), nil) + if h == 0 { + m.restoreCommitted(addr, size) + return err + } + if _, err := mapViewOfFile3(h, addr, uint64(fileOff), size, + _MEM_REPLACE_PLACEHOLDER, _PAGE_READWRITE); err != nil { + windows.CloseHandle(h) + m.restoreCommitted(addr, size) + return err + } + // The view keeps the section alive; the handle is no longer needed. + windows.CloseHandle(h) + m.views = append(m.views, addrOff) + return nil +} + +// restoreCommitted turns a placeholder back into committed private memory. +// It must not fail: a range of the linear memory would otherwise be left +// PAGE_NOACCESS, and whatever wasm code owns it would fault on next touch. +// Callers rely on the invariant that MapFileRegion/UnmapFileRegion return +// with the address space intact. +func (m *Memory) restoreCommitted(addr, size uintptr) { + if _, err := virtualAlloc2(addr, size, + _MEM_RESERVE|_MEM_COMMIT|_MEM_REPLACE_PLACEHOLDER, _PAGE_READWRITE); err != nil { + panic(err) + } +} + +// UnmapFileRegion releases a view created by MapFileRegion and restores +// committed private (zeroed) memory in its place. +func (m *Memory) UnmapFileRegion(addrOff, size uintptr) error { + addr := m.ptr + addrOff + if err := unmapViewOfFile2(addr, _MEM_PRESERVE_PLACEHOLDER); err != nil { + // The view is still mapped; the address space is intact. + return err + } + // The view is gone: record that before anything else, + // so Close never unmaps this offset a second time. + for i, off := range m.views { + if off == addrOff { + m.views = append(m.views[:i], m.views[i+1:]...) + break + } + } + m.restoreCommitted(addr, size) + return nil +} + func (m *Memory) Close() error { - err := windows.VirtualFree(m.ptr, 0, windows.MEM_RELEASE) + if m.ptr == 0 { + m.Buf = nil + return nil + } + var err error + if m.placeh { + // Unmap views first, then release every allocation and the + // remaining placeholders piecewise. + for _, off := range m.views { + unmapViewOfFile2(m.ptr+off, 0) + } + // Walk the region, releasing each allocation. + addr := m.ptr + end := m.ptr + uintptr(cap(m.Buf)) + for addr < end { + var info windows.MemoryBasicInformation + if e := windows.VirtualQuery(addr, &info, unsafe.Sizeof(info)); e != nil { + // Cannot advance without RegionSize; the rest leaks. + if err == nil { + err = e + } + break + } + if info.State != _MEM_FREE { + if e := windows.VirtualFree(info.AllocationBase, 0, windows.MEM_RELEASE); e != nil && err == nil { + err = e + } + } + next := info.BaseAddress + info.RegionSize + if next <= addr { + break + } + addr = next + } + } else { + err = windows.VirtualFree(m.ptr, 0, windows.MEM_RELEASE) + } m.Buf = nil m.com = 0 m.ptr = 0 + m.views = nil return err } diff --git a/internal/sqlite3_wrap/mmap_windows.go b/internal/sqlite3_wrap/mmap_windows.go deleted file mode 100644 index 46e5ac48..00000000 --- a/internal/sqlite3_wrap/mmap_windows.go +++ /dev/null @@ -1,52 +0,0 @@ -package sqlite3_wrap - -import ( - "os" - "unsafe" - - "golang.org/x/sys/windows" -) - -type MappedRegion struct { - Data []byte - addr uintptr -} - -func MapRegion(f *os.File, offset int64, size int32) (*MappedRegion, error) { - maxSize := offset + int64(size) - h, err := windows.CreateFileMapping( - windows.Handle(f.Fd()), nil, windows.PAGE_READWRITE, - uint32(maxSize>>32), uint32(maxSize), nil) - if h == 0 { - return nil, err - } - defer windows.CloseHandle(h) - - const allocationGranularity = 64 * 1024 - align := offset % allocationGranularity - offset -= align - - a, err := windows.MapViewOfFile(h, windows.FILE_MAP_WRITE, - uint32(offset>>32), uint32(offset), uintptr(size)+uintptr(align)) - if a == 0 { - return nil, err - } - - ptr := *(*unsafe.Pointer)(unsafe.Pointer(&a)) - return &MappedRegion{ - Data: unsafe.Slice((*byte)(unsafe.Add(ptr, align)), size), - addr: a, - }, nil -} - -func (r *MappedRegion) Unmap() error { - if r.Data == nil { - return nil - } - err := windows.UnmapViewOfFile(r.addr) - if err != nil { - return err - } - r.Data = nil - return nil -} diff --git a/internal/sqlite3_wrap/placeholder_windows.go b/internal/sqlite3_wrap/placeholder_windows.go new file mode 100644 index 00000000..786e8d42 --- /dev/null +++ b/internal/sqlite3_wrap/placeholder_windows.go @@ -0,0 +1,93 @@ +package sqlite3_wrap + +// Address-space placeholders (Windows 10 1803+ / Server 2019+) let a file +// view be mapped INTO the wasm linear memory, the same way the unix build +// maps the WAL-index with MAP_FIXED. SQLite then works on genuinely shared +// memory: no private copies, no sync points, native memory semantics. +// +// The round-trip used here: +// +// reserve: VirtualAlloc2(MEM_RESERVE|MEM_RESERVE_PLACEHOLDERS) +// commit: split placeholder, VirtualAlloc2(MEM_REPLACE_PLACEHOLDER|COMMIT) +// carve: split a committed (replaced-placeholder) range back into a +// placeholder with VirtualFree(MEM_RELEASE|MEM_PRESERVE_PLACEHOLDER) +// map: MapViewOfFile3(MEM_REPLACE_PLACEHOLDER) into the carved hole +// unmap: UnmapViewOfFile2(MEM_PRESERVE_PLACEHOLDER), then re-commit + +import ( + "golang.org/x/sys/windows" +) + +const ( + _MEM_COMMIT = 0x00001000 + _MEM_RESERVE = 0x00002000 + _MEM_RELEASE = 0x00008000 + _MEM_FREE = 0x00010000 + _MEM_RESERVE_PLACEHOLDERS = 0x00040000 + _MEM_REPLACE_PLACEHOLDER = 0x00004000 + _MEM_PRESERVE_PLACEHOLDER = 0x00000002 + _PAGE_READWRITE = 0x04 + _PAGE_NOACCESS = 0x01 +) + +var ( + kernelbase = windows.NewLazySystemDLL("kernelbase.dll") + procVirtualAlloc2 = kernelbase.NewProc("VirtualAlloc2") + procMapViewOfFile3 = kernelbase.NewProc("MapViewOfFile3") + procUnmapViewOfFile2 = kernelbase.NewProc("UnmapViewOfFile2") +) + +// PlaceholdersSupported reports whether this Windows version has the +// placeholder APIs (Windows 10 1803+ / Server 2019+). +func PlaceholdersSupported() bool { + return procVirtualAlloc2.Find() == nil && + procMapViewOfFile3.Find() == nil && + procUnmapViewOfFile2.Find() == nil +} + +func virtualAlloc2(addr uintptr, size uintptr, allocType, protect uint32) (uintptr, error) { + r, _, err := procVirtualAlloc2.Call( + 0, // current process + addr, + size, + uintptr(allocType), + uintptr(protect), + 0, 0) // no extended parameters + if r == 0 { + return 0, err + } + return r, nil +} + +func mapViewOfFile3(h windows.Handle, addr uintptr, offset uint64, size uintptr, allocType, protect uint32) (uintptr, error) { + r, _, err := procMapViewOfFile3.Call( + uintptr(h), + 0, // current process + addr, + uintptr(offset), + size, + uintptr(allocType), + uintptr(protect), + 0, 0) + if r == 0 { + return 0, err + } + return r, nil +} + +func unmapViewOfFile2(addr uintptr, unmapFlags uint32) error { + r, _, err := procUnmapViewOfFile2.Call( + ^uintptr(0), // current process pseudo handle + addr, + uintptr(unmapFlags)) + if r == 0 { + return err + } + return nil +} + +// splitPlaceholder shrinks the placeholder/allocation containing +// [addr, addr+size) to exactly that range, so it can be replaced. +func splitPlaceholder(addr, size uintptr) error { + return windows.VirtualFree(addr, size, _MEM_RELEASE|_MEM_PRESERVE_PLACEHOLDER) +} diff --git a/vfs/shm_copy.go b/vfs/shm_copy.go index 29a8c340..31859ccd 100644 --- a/vfs/shm_copy.go +++ b/vfs/shm_copy.go @@ -1,9 +1,10 @@ -//go:build windows || sqlite3_dotlk +//go:build sqlite3_dotlk package vfs import ( - "sync/atomic" + "bytes" + "sync" "unsafe" ) @@ -12,33 +13,103 @@ const ( _WALINDEX_PGSZ = 32768 ) -// This seems a safe way of keeping the WAL-index in sync. +// The wal-index is kept in sync by copying at lock boundaries: +// acquire copies shared→private after a lock is taken, +// release copies private→shared before an exclusive lock is dropped, +// and a barrier does both. A per-connection shadow of the shared memory +// lets both directions copy only the words that actually changed. // -// The WAL-index file starts with a header, -// and the index doesn't meaningfully change if the header doesn't change. -// -// The header starts with two 48 byte, checksummed, copies of the same information, -// which are accessed independently between memory barriers. -// The checkpoint information that follows uses 4 byte aligned words. +// https://sqlite.org/walformat.html#the_wal_index_file_format // -// Finally, we have the WAL-index hash tables, -// which are only modified holding the exclusive WAL_WRITE_LOCK. +// Correctness requires two properties that plain word-copy loops do not +// provide on their own: // -// Since all the data is either redundant+checksummed, -// 4 byte aligned, or modified under an exclusive lock, -// the copies below should correctly keep memory in sync. +// 1. Copy operations of different connections must not interleave. +// They are triggered by locks on *different* wal-index lock bytes +// (e.g. a writer releasing WAL_WRITE_LOCK while a reader acquires a +// READ_LOCK), so the file locks do not order them. An acquire that +// overlaps a release can capture a fresh header together with +// hole-ridden hash tables: SQLite then trusts hash lookups that are +// missing frames, and a checkpointer acting on such a view copies +// stale page versions into the database and truncates frames that +// were never backfilled. A per-file lock (shmCopyLocks, keyed by the +// -shm path) serializes all copies for that file; connections to +// different files share no wal-index state and need no ordering. +// (Native SQLite needs no such lock only because all connections read +// and write one coherent mapping; its winShmNode similarly arbitrates +// same-process access through shared state.) // -// https://sqlite.org/walformat.html#the_wal_index_file_format +// 2. A "nothing changed" fast path must compare exactly. The header +// (first 136 bytes) is NOT a proxy for the whole page: SQLite +// modifies hash-table words while leaving the header byte-identical — +// e.g. a rolled-back transaction zeroes its hash entries via +// walCleanupHash and restores the header — so a header-only check +// skips real changes, the private and shared copies diverge +// permanently, and after a checkpoint restart reuses frame numbers +// the stale entries alias wrong frames. The skip below compares the +// full page instead; bytes.Equal costs ~1µs per 32K page, which the +// shadow-diff design already tolerates. +type shmCopyLock struct { + sync.Mutex + refs int // +checklocks:shmCopyLocksMtx +} + +var ( + // +checklocks:shmCopyLocksMtx + shmCopyLocks = map[string]*shmCopyLock{} + shmCopyLocksMtx sync.Mutex +) + +func shmCopyLockGet(path string) *shmCopyLock { + shmCopyLocksMtx.Lock() + defer shmCopyLocksMtx.Unlock() + l := shmCopyLocks[path] + if l == nil { + l = &shmCopyLock{} + shmCopyLocks[path] = l + } + l.refs++ + return l +} + +func shmCopyLockPut(path string) { + shmCopyLocksMtx.Lock() + defer shmCopyLocksMtx.Unlock() + if l := shmCopyLocks[path]; l != nil { + if l.refs--; l.refs <= 0 { + delete(shmCopyLocks, path) + } + } +} func (s *vfsShm) shmAcquire(errp *error) { if errp != nil && *errp != nil { return } - if len(s.ptrs) == 0 || shmEqual(s.shadow[0][:], s.shared[0][:]) { - return + if s.copyMu == nil { + return // no copy state yet (no pages mapped) + } + s.copyMu.Lock() + defer s.copyMu.Unlock() + s.shmAcquireLocked() +} + +func (s *vfsShm) shmRelease() { + if s.copyMu == nil { + return // no copy state yet (no pages mapped) } - // Copies modified words from shared to private memory. + s.copyMu.Lock() + defer s.copyMu.Unlock() + s.shmReleaseLocked() +} + +// shmAcquireLocked copies modified words from shared to private memory. +// Callers must hold the file's copy lock. +func (s *vfsShm) shmAcquireLocked() { for id, p := range s.ptrs { + if bytes.Equal(s.shadow[id][:], s.shared[id][:]) { + continue // page unchanged since this connection last synced + } shared := shmPage(s.shared[id][:]) shadow := shmPage(s.shadow[id][:]) privat := shmPage(s.wrp.Bytes(p, _WALINDEX_PGSZ)) @@ -51,12 +122,13 @@ func (s *vfsShm) shmAcquire(errp *error) { } } -func (s *vfsShm) shmRelease() { - if len(s.ptrs) == 0 || shmEqual(s.shadow[0][:], s.wrp.Bytes(s.ptrs[0], _WALINDEX_HDR_SIZE)) { - return - } - // Copies modified words from private to shared memory. +// shmReleaseLocked copies modified words from private to shared memory. +// Callers must hold the file's copy lock. +func (s *vfsShm) shmReleaseLocked() { for id, p := range s.ptrs { + if bytes.Equal(s.shadow[id][:], s.wrp.Bytes(p, _WALINDEX_PGSZ)) { + continue // this connection made no changes to this page + } shared := shmPage(s.shared[id][:]) shadow := shmPage(s.shadow[id][:]) privat := shmPage(s.wrp.Bytes(p, _WALINDEX_PGSZ)) @@ -69,22 +141,8 @@ func (s *vfsShm) shmRelease() { } } -func (s *vfsShm) shmBarrier() { - var b atomic.Bool - s.Lock() - s.shmAcquire(nil) - b.Swap(true) - s.shmRelease() - s.Unlock() -} - //go:nosplit func shmPage(s []byte) *[_WALINDEX_PGSZ / 4]uint32 { p := (*uint32)(unsafe.Pointer(unsafe.SliceData(s))) return (*[_WALINDEX_PGSZ / 4]uint32)(unsafe.Slice(p, _WALINDEX_PGSZ/4)) } - -//go:nosplit -func shmEqual(v1, v2 []byte) bool { - return *(*[_WALINDEX_HDR_SIZE]byte)(v1[:]) == *(*[_WALINDEX_HDR_SIZE]byte)(v2[:]) -} diff --git a/vfs/shm_dotlk.go b/vfs/shm_dotlk.go index 3860d91d..bb21ba96 100644 --- a/vfs/shm_dotlk.go +++ b/vfs/shm_dotlk.go @@ -30,6 +30,7 @@ type vfsShm struct { *vfsShmParent wrp *sqlite3_wrap.Wrapper path string + copyMu *shmCopyLock // serializes copies per shared buffer shadow [][_WALINDEX_PGSZ]byte ptrs []ptr_t lock [_SHM_NLOCK]bool @@ -40,6 +41,12 @@ func (s *vfsShm) Close() error { return nil } + // Release the per-file copy lock; nothing after this copies. + if s.copyMu != nil { + shmCopyLockPut(s.path) + s.copyMu = nil + } + vfsShmListMtx.Lock() defer vfsShmListMtx.Unlock() @@ -73,6 +80,7 @@ func (s *vfsShm) shmOpen() error { if g, ok := vfsShmList[s.path]; ok { s.vfsShmParent = g g.refs++ + s.copyMu = shmCopyLockGet(s.path) return nil } @@ -88,6 +96,7 @@ func (s *vfsShm) shmOpen() error { // Add the new shared buffer. s.vfsShmParent = &vfsShmParent{} vfsShmList[s.path] = s.vfsShmParent + s.copyMu = shmCopyLockGet(s.path) return nil } @@ -133,6 +142,18 @@ func (s *vfsShm) shmMap(wrp *sqlite3_wrap.Wrapper, id, size int32, extend bool) return s.ptrs[id], nil } +func (s *vfsShm) shmBarrier() { + if s.copyMu == nil { + return // never mapped + } + s.Lock() + s.copyMu.Lock() + s.shmAcquireLocked() + s.shmReleaseLocked() + s.copyMu.Unlock() + s.Unlock() +} + func (s *vfsShm) shmLock(offset, n int32, flags _ShmFlag) (err error) { if s.vfsShmParent == nil { return _IOERR_SHMLOCK diff --git a/vfs/shm_windows.go b/vfs/shm_windows.go index 482f79b0..b9d8ed1c 100644 --- a/vfs/shm_windows.go +++ b/vfs/shm_windows.go @@ -5,7 +5,7 @@ package vfs import ( "io" "os" - "sync" + "sync/atomic" "golang.org/x/sys/windows" @@ -13,24 +13,61 @@ import ( "github.com/ncruces/go-sqlite3/internal/sqlite3_wrap" ) +// On Windows 10 1803+ / Server 2019+ the WAL-index is mapped directly into +// the wasm linear memory (like the unix build maps it with MAP_FIXED): +// SQLite then works on genuinely shared, always-current memory, which +// wal.c's protocol assumes. +// +// wal.c reads and writes the index BETWEEN xShmLock calls and relies on +// seeing other connections' updates live — e.g. the checkpointer probes each +// aReadMark slot and, when the probe loses to a live reader, trusts the mark +// value it read moments earlier. Any scheme that synchronizes copies of the +// index only at lock boundaries serves stale views on exactly such paths; +// that reliably corrupted databases under concurrent write load (torn reads +// of database pages being backfilled past a reader's snapshot, checkpoints +// driven by hole-ridden hash tables, ...). +// +// Below Windows 10 1803 the placeholder APIs are unavailable, so the index +// cannot be mapped into linear memory. Rather than fall back to a +// copy-on-lock-boundary scheme that cannot make wal.c's between-lock reads +// coherent, shared-memory WAL is refused there: such a build must use +// [WAL without shared-memory] via [EXCLUSIVE locking mode], the same as +// platforms without shared-memory support. +// +// [WAL without shared-memory]: https://sqlite.org/wal.html#noshm +// [EXCLUSIVE locking mode]: https://sqlite.org/pragma.html#pragma_locking_mode + +const ( + _WALINDEX_PGSZ = 32768 + _SHM_VIEW = 2 * _WALINDEX_PGSZ // regions are mapped in 64K pairs +) + +type shmView struct { + block ptr_t // sqlite3_malloc'd block the view was carved from + addr ptr_t // 64K-aligned start of the mapped 64K view +} + type vfsShm struct { *os.File wrp *sqlite3_wrap.Wrapper path string - regions []*sqlite3_wrap.MappedRegion - shared [][]byte - shadow [][_WALINDEX_PGSZ]byte - ptrs []ptr_t + views []shmView // one per mapped region pair fileLock bool - sync.Mutex } func (s *vfsShm) Close() error { - // Unmap regions. - for _, r := range s.regions { - r.Unmap() + // Unmap views. + for _, v := range s.views { + if err := s.wrp.UnmapFileRegion(uintptr(v.addr), _SHM_VIEW); err != nil { + // The view is still mapped over the block: freeing it would + // let the allocator hand out file-backed memory, and writes + // through it would corrupt the wal-index for every other + // connection. Leak the block instead. + continue + } + s.wrp.Xsqlite3_free(int32(v.block)) } - s.regions = nil + s.views = nil // Close the file. return s.File.Close() @@ -70,15 +107,26 @@ func (s *vfsShm) shmMap(wrp *sqlite3_wrap.Wrapper, id, size int32, extend bool) if s.wrp == nil { s.wrp = wrp } + // Below Windows 10 1803 the index cannot be mapped into linear memory; + // refuse shared-memory WAL (WAL then needs EXCLUSIVE locking mode). + if !wrp.CanMapFiles() { + return 0, _IOERR_SHMMAP + } if err := s.shmOpen(); err != nil { return 0, err } + return s.shmMapDirect(id, size, extend) +} - s.Lock() - defer s.Unlock() - defer s.shmAcquire(&err) +// shmMapDirect maps the region into the wasm linear memory, in 64K pairs +// (both the map address and the file offset must be allocation-granularity +// aligned). The view is carved out of a sqlite3_malloc'd block: the +// allocator keeps the address range reserved while the underlying pages are +// replaced by the file view. +func (s *vfsShm) shmMapDirect(id, size int32, extend bool) (ptr_t, error) { + pair := int(id) / 2 - // Check if file is big enough. + // Check if the file covers the requested region. o, err := s.Seek(0, io.SeekEnd) if err != nil { return 0, sysError{err, _IOERR_SHMSIZE} @@ -87,38 +135,41 @@ func (s *vfsShm) shmMap(wrp *sqlite3_wrap.Wrapper, id, size int32, extend bool) if !extend { return 0, nil } - if err := osAllocate(s.File, n); err != nil { - return 0, sysError{err, _IOERR_SHMSIZE} - } } - // Maps regions into memory. - for int(id) >= len(s.shared) { - r, err := sqlite3_wrap.MapRegion(s.File, int64(id)*int64(size), size) - if err != nil { - return 0, err + for len(s.views) <= pair { + p := len(s.views) + // The pair's view spans [p*64K, (p+1)*64K) of the file. + // This can grow the file to the pair boundary even when + // extend is false (e.g. a 32K file, region 0 requested), + // which deviates from the xShmMap contract. It is benign: + // the extension is zero-filled, zero hash pages read as + // empty, and no reader consults index pages beyond the + // mxFrame published in the header — the same state a + // legitimate writer extension leaves before publishing. + if n := int64(p+1) * _SHM_VIEW; n > o { + if err := osAllocate(s.File, n); err != nil { + return 0, sysError{err, _IOERR_SHMSIZE} + } } - s.regions = append(s.regions, r) - s.shared = append(s.shared, r.Data) - } - - // Allocate shadow memory. - if int(id) >= len(s.shadow) { - s.shadow = append(s.shadow, make([][_WALINDEX_PGSZ]byte, int(id)-len(s.shadow)+1)...) - } - - // Allocate local memory. - for int(id) >= len(s.ptrs) { - ptr := wrp.Xsqlite3_malloc64(int64(size)) - if ptr == 0 { + // Carve a 64K-aligned 64K hole from a malloc'd block + // (2×64K guarantees an aligned 64K fits at any block offset). + block := s.wrp.Xsqlite3_malloc64(4 * _WALINDEX_PGSZ) + if block == 0 { panic(errutil.OOMErr) } - clear(wrp.Bytes(ptr_t(ptr), _WALINDEX_PGSZ)) - s.ptrs = append(s.ptrs, ptr_t(ptr)) + // Convert through uint32: the wasm pointer is returned as int32, + // and above 2 GiB of linear memory bit 31 is set — a direct + // uintptr conversion would sign-extend and wreck the arithmetic. + aligned := (uintptr(uint32(block)) + _SHM_VIEW - 1) &^ (_SHM_VIEW - 1) + if err := s.wrp.MapFileRegion(s.File, int64(p)*_SHM_VIEW, aligned, _SHM_VIEW); err != nil { + s.wrp.Xsqlite3_free(int32(block)) + return 0, sysError{err, _IOERR_SHMMAP} + } + s.views = append(s.views, shmView{block: ptr_t(block), addr: ptr_t(aligned)}) } - s.shadow[0][4] = 1 - return s.ptrs[id], nil + return s.views[pair].addr + ptr_t(int(id)%2)*_WALINDEX_PGSZ, nil } func (s *vfsShm) shmLock(offset, n int32, flags _ShmFlag) (err error) { @@ -126,16 +177,6 @@ func (s *vfsShm) shmLock(offset, n int32, flags _ShmFlag) (err error) { return _IOERR_SHMLOCK } - s.Lock() - defer s.Unlock() - - switch { - case flags&_SHM_LOCK != 0: - defer s.shmAcquire(&err) - case flags&_SHM_EXCLUSIVE != 0: - s.shmRelease() - } - switch { case flags&_SHM_UNLOCK != 0: return osUnlock(s.File, _SHM_BASE+uint32(offset), uint32(n)) @@ -153,19 +194,7 @@ func (s *vfsShm) shmUnmap(delete bool) { return } - s.Lock() - s.shmRelease() - defer s.Unlock() - - // Free local memory. - for _, p := range s.ptrs { - s.wrp.Xsqlite3_free(int32(p)) - } - s.ptrs = nil - s.shadow = nil - s.shared = nil - - // Close the file. + // Close the file (also unmaps views). s.Close() s.File = nil s.fileLock = false @@ -173,3 +202,9 @@ func (s *vfsShm) shmUnmap(delete bool) { os.Remove(s.path) } } + +func (s *vfsShm) shmBarrier() { + // The index is genuinely shared: a memory fence suffices, as on unix. + var b atomic.Bool + b.Swap(true) +} From 8645d9736f69e332d100379599d06dd497abe118 Mon Sep 17 00:00:00 2001 From: franchb Date: Tue, 7 Jul 2026 01:52:07 -0500 Subject: [PATCH 2/2] vfs: add a concurrent WAL writers regression test for #404 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A repeat-until-budget stress test that drives many concurrent WAL writers through one *sql.DB, aggressively checkpoints (TRUNCATE), then cold-reopens the file and runs PRAGMA integrity_check. It repeats that cycle until it either observes the defect or a time budget elapses. On the pre-fix copy-on-lock-boundary Windows scheme this reliably corrupts the database within the first round (database disk image malformed / file is not a database / SQLITE_PROTOCOL). On the fix — and on every platform whose VFS maps the -shm directly (unix, native) — every round stays clean. The test therefore goes red only on an unfixed Windows build and green everywhere else, so it can be watched to flip red→green. It is opt-in: several minutes of heavy I/O, gated behind SQLITE3_TEST_WAL_STRESS so it never taxes the normal test matrix. The wal-repro workflow (workflow_dispatch) flips the gate and runs it on Windows and Linux on demand. BUSY, IOERR and FULL are tolerated during the storm: heavy concurrency and a full or slow scratch filesystem are environment limits, not the defect, and the defect surfaces only as SQLITE_PROTOCOL / CORRUPT / NOTADB. The cold-reopen integrity_check is the definitive corruption gate regardless. The workload (64 writers, 2000 iters, 8K rows, checkpoint every 25) is sized to open the race window even on the slow temp disk of hosted CI runners; lighter workloads under-expose it and can false-green. --- .github/workflows/wal-repro.yml | 27 ++++ vfs/tests/doc.go | 2 + vfs/tests/wal_stress_test.go | 213 ++++++++++++++++++++++++++++++++ 3 files changed, 242 insertions(+) create mode 100644 .github/workflows/wal-repro.yml create mode 100644 vfs/tests/doc.go create mode 100644 vfs/tests/wal_stress_test.go diff --git a/.github/workflows/wal-repro.yml b/.github/workflows/wal-repro.yml new file mode 100644 index 00000000..648ffb54 --- /dev/null +++ b/.github/workflows/wal-repro.yml @@ -0,0 +1,27 @@ +name: WAL Repro + +# On-demand regression run for the Windows WAL-index corruption. The test is +# opt-in (gated behind SQLITE3_TEST_WAL_STRESS) so it never taxes the normal +# test matrix. On an unfixed Windows build it fails in the first round; on the +# fixed build, and on every direct-mapped VFS (unix, native), it stays green. +on: + workflow_dispatch: + +permissions: + contents: read + +jobs: + wal: + strategy: + fail-fast: false + matrix: + os: [windows-latest, ubuntu-latest] + runs-on: ${{ matrix.os }} + env: + SQLITE3_TEST_WAL_STRESS: "1" + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-go@v6 + with: { go-version: stable } + - name: WAL concurrent-writers regression + run: go test ./vfs/tests/ -run '^TestWALConcurrentWriters$' -v -count=1 -timeout 25m diff --git a/vfs/tests/doc.go b/vfs/tests/doc.go new file mode 100644 index 00000000..21883a8f --- /dev/null +++ b/vfs/tests/doc.go @@ -0,0 +1,2 @@ +// Package tests holds heavy, end-to-end VFS stress tests. +package tests diff --git a/vfs/tests/wal_stress_test.go b/vfs/tests/wal_stress_test.go new file mode 100644 index 00000000..841d399d --- /dev/null +++ b/vfs/tests/wal_stress_test.go @@ -0,0 +1,213 @@ +// The sqlite3_dotlk build keeps the copy-on-lock-boundary scheme, which +// cannot fully eliminate wal-index staleness: under this load it reliably +// fails with SQLITE_PROTOCOL (a documented limitation, not corruption — +// the database stays intact). Only the real-file VFS paths are exercised. +// +//go:build !sqlite3_dotlk + +package tests + +import ( + "context" + "crypto/rand" + "database/sql" + "errors" + "fmt" + "os" + "path/filepath" + "runtime" + "sync" + "testing" + "time" + + sqlite3 "github.com/ncruces/go-sqlite3" + "github.com/ncruces/go-sqlite3/driver" +) + +// TestWALConcurrentWriters drives many concurrent connections writing to a +// single WAL database through one *sql.DB, aggressively checkpoints, then +// cold-reopens the file and runs PRAGMA integrity_check. It repeats that whole +// cycle until either the defect is observed or a time budget elapses. +// +// It is a regression test for Windows WAL-index corruption. On the +// copy-on-lock-boundary shared-memory scheme (the Windows default before the +// -shm was mapped into wasm memory) this corrupts the database on Windows: a +// checkpointer acting on a stale wal-index view backfills stale pages and +// truncates not-yet-backfilled frames. The cold-reopen integrity_check catches +// it, and SQLITE_PROTOCOL during the run is the other symptom of the same bug. +// +// On the fixed VFS — and on every platform whose VFS maps the -shm directly +// (unix, native) — every round stays clean. The test therefore goes red only +// on an unfixed Windows build and green everywhere else, which is exactly what +// a regression gate should do. +// +// BUSY is tolerated: under this many writers, busy_timeout exhaustion is +// ordinary backpressure, not the defect. IOERR and FULL are tolerated too: +// this drives GBs of writes through a scratch directory, and a full or +// slow temp filesystem is an environment limit, not corruption — the +// cold-reopen integrity_check remains the definitive corruption gate. The +// defect surfaces as SQLITE_PROTOCOL, "malformed", or "not a database", +// none of which are BUSY/IOERR/FULL. Excluded from sqlite3_dotlk builds +// (see the build constraint). +// +// It is opt-in — several minutes of heavy I/O — so it does not tax the normal +// test run: set SQLITE3_TEST_WAL_STRESS=1 to enable it (the wal-repro workflow +// does this). On an unfixed Windows build it reproduces in the first round, +// usually within a minute. +func TestWALConcurrentWriters(t *testing.T) { + if testing.Short() || os.Getenv("SQLITE3_TEST_WAL_STRESS") == "" { + t.Skip("opt-in heavy WAL concurrent-writers regression; " + + "set SQLITE3_TEST_WAL_STRESS=1 to run") + } + + const ( + workers = 64 + iters = 2000 + blobBytes = 8192 + ckptEvery = 25 + maxRounds = 3 + budget = 7 * time.Minute + ) + t.Logf("runner parallelism: NumCPU=%d GOMAXPROCS=%d", runtime.NumCPU(), runtime.GOMAXPROCS(0)) + + blob := make([]byte, blobBytes) + if _, err := rand.Read(blob); err != nil { + t.Fatalf("rand: %v", err) + } + + deadline := time.Now().Add(budget) + round := 0 + for round < maxRounds { + round++ + if bad := walStressRound(t, workers, iters, ckptEvery, blob); bad != "" { + t.Fatalf("round %d: WAL corruption reproduced: %s", round, bad) + } + if time.Now().After(deadline) { + break + } + } + t.Logf("%d concurrent-writer rounds clean (workers=%d iters=%d)", round, workers, iters) +} + +// walStressRound runs one storm+checkpoint cycle in a fresh database and +// returns a non-empty description if the WAL defect was observed. It removes +// its own scratch directory so repeated rounds do not accumulate disk. +func walStressRound(t *testing.T, workers, iters, ckptEvery int, blob []byte) string { + t.Helper() + dir, err := os.MkdirTemp("", "wal_stress") + if err != nil { + t.Fatalf("mkdtemp: %v", err) + } + defer os.RemoveAll(dir) + + path := filepath.Join(dir, "wal_stress.db") + dsn := "file:" + path + + "?_pragma=busy_timeout(10000)&_pragma=journal_mode(wal)" + + "&_pragma=synchronous(normal)&_txlock=deferred" + + db, err := driver.Open(dsn) + if err != nil { + t.Fatalf("open: %v", err) + } + db.SetMaxOpenConns(workers) + if _, err := db.Exec(` + CREATE TABLE parent(id INTEGER PRIMARY KEY AUTOINCREMENT, hash TEXT NOT NULL UNIQUE); + CREATE TABLE child(id INTEGER PRIMARY KEY AUTOINCREMENT, + parent_id INTEGER NOT NULL REFERENCES parent(id) ON DELETE CASCADE, data BLOB NOT NULL); + CREATE INDEX child_parent_idx ON child(parent_id); + `); err != nil { + db.Close() + t.Fatalf("schema: %v", err) + } + + var wg sync.WaitGroup + errCh := make(chan error, workers) + for w := 0; w < workers; w++ { + wg.Add(1) + go func(gid int) { + defer wg.Done() + for i := 0; i < iters; i++ { + if err := doTx(db, gid, i, blob); err != nil { + if isBackpressure(err) { + continue // backpressure or environment limit, not the defect + } + errCh <- fmt.Errorf("worker %d iter %d: %w", gid, i, err) + return + } + if i%ckptEvery == 0 { + if _, err := db.Exec("PRAGMA wal_checkpoint(TRUNCATE)"); err != nil && + !isBackpressure(err) { + errCh <- fmt.Errorf("checkpoint: %w", err) + return + } + } + } + }(w) + } + wg.Wait() + close(errCh) + var workErr error + for err := range errCh { + workErr = err + break + } + if err := db.Close(); err != nil { + t.Fatalf("close: %v", err) + } + + // A worker error that is not backpressure/environment (SQLITE_PROTOCOL + // "locking protocol", "malformed", "file is not a database") is a defect + // symptom, not infrastructure. + if workErr != nil { + return fmt.Sprintf("worker error: %v", workErr) + } + + // Cold reopen + integrity gate — the definitive assertion. + db2, err := driver.Open(dsn) + if err != nil { + return fmt.Sprintf("cold reopen failed: %v", err) + } + defer db2.Close() + db2.SetMaxOpenConns(1) + var first string + if err := db2.QueryRow("PRAGMA integrity_check").Scan(&first); err != nil { + return fmt.Sprintf("integrity_check errored: %v", err) + } + if first != "ok" { + return fmt.Sprintf("integrity_check = %q", first) + } + return "" +} + +// isBackpressure reports whether err is ordinary write backpressure (BUSY) +// or an environment limit (a full or failing scratch filesystem: IOERR, +// FULL) rather than a symptom of the WAL defect. The defect surfaces as +// SQLITE_PROTOCOL, CORRUPT ("malformed"), or NOTADB ("not a database"), +// none of which match these codes; the cold-reopen integrity_check is the +// definitive corruption gate regardless. +func isBackpressure(err error) bool { + return errors.Is(err, sqlite3.BUSY) || + errors.Is(err, sqlite3.IOERR) || + errors.Is(err, sqlite3.FULL) +} + +func doTx(db *sql.DB, gid, i int, blob []byte) error { + ctx := context.Background() + tx, err := db.BeginTx(ctx, nil) + if err != nil { + return err + } + var pid int64 + if err := tx.QueryRowContext(ctx, + "INSERT INTO parent(hash) VALUES(?) RETURNING id", + fmt.Sprintf("%d-%d", gid, i)).Scan(&pid); err != nil { + _ = tx.Rollback() + return err + } + if _, err := tx.ExecContext(ctx, + "INSERT INTO child(parent_id, data) VALUES(?, ?)", pid, blob); err != nil { + _ = tx.Rollback() + return err + } + return tx.Commit() +}