Fix WAL flakiness on Windows (#253)

util/osutil/open.go

@@ -1,16 +0,0 @@
-//go:build !windows
-
-package osutil
-
-import (
-	"io/fs"
-	"os"
-)
-
-// OpenFile behaves the same as [os.OpenFile],
-// except on Windows it sets [syscall.FILE_SHARE_DELETE].
-//
-// See: https://go.dev/issue/32088#issuecomment-502850674
-func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
-	return os.OpenFile(name, flag, perm)
-}

util/osutil/open_windows.go

@@ -1,126 +0,0 @@
-package osutil
-
-import (
-	"io/fs"
-	"os"
-	. "syscall"
-	"unsafe"
-)
-
-// OpenFile behaves the same as [os.OpenFile],
-// except on Windows it sets [syscall.FILE_SHARE_DELETE].
-//
-// See: https://go.dev/issue/32088#issuecomment-502850674
-func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
-	if name == "" {
-		return nil, &os.PathError{Op: "open", Path: name, Err: ENOENT}
-	}
-	r, err := syscallOpen(name, flag|O_CLOEXEC, uint32(perm.Perm()))
-	if err != nil {
-		return nil, &os.PathError{Op: "open", Path: name, Err: err}
-	}
-	return os.NewFile(uintptr(r), name), nil
-}
-
-// syscallOpen is a copy of [syscall.Open]
-// that uses [syscall.FILE_SHARE_DELETE],
-// and supports [syscall.FILE_FLAG_OVERLAPPED].
-//
-// https://go.dev/src/syscall/syscall_windows.go
-func syscallOpen(name string, flag int, perm uint32) (fd Handle, err error) {
-	if len(name) == 0 {
-		return InvalidHandle, ERROR_FILE_NOT_FOUND
-	}
-	namep, err := UTF16PtrFromString(name)
-	if err != nil {
-		return InvalidHandle, err
-	}
-	var access uint32
-	switch flag & (O_RDONLY | O_WRONLY | O_RDWR) {
-	case O_RDONLY:
-		access = GENERIC_READ
-	case O_WRONLY:
-		access = GENERIC_WRITE
-	case O_RDWR:
-		access = GENERIC_READ | GENERIC_WRITE
-	}
-	if flag&O_CREAT != 0 {
-		access |= GENERIC_WRITE
-	}
-	if flag&O_APPEND != 0 {
-		// Remove GENERIC_WRITE unless O_TRUNC is set, in which case we need it to truncate the file.
-		// We can't just remove FILE_WRITE_DATA because GENERIC_WRITE without FILE_WRITE_DATA
-		// starts appending at the beginning of the file rather than at the end.
-		if flag&O_TRUNC == 0 {
-			access &^= GENERIC_WRITE
-		}
-		// Set all access rights granted by GENERIC_WRITE except for FILE_WRITE_DATA.
-		access |= FILE_APPEND_DATA | FILE_WRITE_ATTRIBUTES | _FILE_WRITE_EA | STANDARD_RIGHTS_WRITE | SYNCHRONIZE
-	}
-	sharemode := uint32(FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE)
-	var sa *SecurityAttributes
-	if flag&O_CLOEXEC == 0 {
-		sa = makeInheritSa()
-	}
-	// We don't use CREATE_ALWAYS, because when opening a file with
-	// FILE_ATTRIBUTE_READONLY these will replace an existing file
-	// with a new, read-only one. See https://go.dev/issue/38225.
-	//
-	// Instead, we ftruncate the file after opening when O_TRUNC is set.
-	var createmode uint32
-	switch {
-	case flag&(O_CREAT|O_EXCL) == (O_CREAT | O_EXCL):
-		createmode = CREATE_NEW
-	case flag&O_CREAT == O_CREAT:
-		createmode = OPEN_ALWAYS
-	default:
-		createmode = OPEN_EXISTING
-	}
-	var attrs uint32 = FILE_ATTRIBUTE_NORMAL
-	if perm&S_IWRITE == 0 {
-		attrs = FILE_ATTRIBUTE_READONLY
-	}
-	if flag&O_WRONLY == 0 && flag&O_RDWR == 0 {
-		// We might be opening or creating a directory.
-		// CreateFile requires FILE_FLAG_BACKUP_SEMANTICS
-		// to work with directories.
-		attrs |= FILE_FLAG_BACKUP_SEMANTICS
-	}
-	if flag&O_SYNC != 0 {
-		const _FILE_FLAG_WRITE_THROUGH = 0x80000000
-		attrs |= _FILE_FLAG_WRITE_THROUGH
-	}
-	if flag&O_NONBLOCK != 0 {
-		attrs |= FILE_FLAG_OVERLAPPED
-	}
-	h, err := CreateFile(namep, access, sharemode, sa, createmode, attrs, 0)
-	if h == InvalidHandle {
-		if err == ERROR_ACCESS_DENIED && (flag&O_WRONLY != 0 || flag&O_RDWR != 0) {
-			// We should return EISDIR when we are trying to open a directory with write access.
-			fa, e1 := GetFileAttributes(namep)
-			if e1 == nil && fa&FILE_ATTRIBUTE_DIRECTORY != 0 {
-				err = EISDIR
-			}
-		}
-		return h, err
-	}
-	// Ignore O_TRUNC if the file has just been created.
-	if flag&O_TRUNC == O_TRUNC &&
-		(createmode == OPEN_EXISTING || (createmode == OPEN_ALWAYS /*&& err == ERROR_ALREADY_EXISTS*/)) {
-		err = Ftruncate(h, 0)
-		if err != nil {
-			CloseHandle(h)
-			return InvalidHandle, err
-		}
-	}
-	return h, nil
-}
-
-func makeInheritSa() *SecurityAttributes {
-	var sa SecurityAttributes
-	sa.Length = uint32(unsafe.Sizeof(sa))
-	sa.InheritHandle = 1
-	return &sa
-}
-
-const _FILE_WRITE_EA = 0x00000010

util/osutil/osfs.go

@@ -1,3 +1,4 @@
+// Package osutil implements operating system utilities.
 package osutil
 
 import (
@@ -19,7 +20,7 @@ type FS struct{}
 
 // Open implements [fs.FS].
 func (FS) Open(name string) (fs.File, error) {
-	return OpenFile(name, os.O_RDONLY, 0)
+	return os.OpenFile(name, os.O_RDONLY, 0)
 }
 
 // ReadFileFS implements [fs.StatFS].
@@ -31,3 +32,10 @@ func (FS) Stat(name string) (fs.FileInfo, error) {
 func (FS) ReadFile(name string) ([]byte, error) {
 	return os.ReadFile(name)
 }
+
+// OpenFile behaves the same as [os.OpenFile].
+//
+// Deprecated: use os.OpenFile instead.
+func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
+	return os.OpenFile(name, flag, perm)
+}

util/osutil/osutil.go

@@ -1,2 +0,0 @@
-// Package osutil implements operating system utilities.
-package osutil

vfs/file.go

@@ -6,9 +6,8 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"runtime"
 	"syscall"
-
-	"github.com/ncruces/go-sqlite3/util/osutil"
 )
 
 type vfsOS struct{}
@@ -40,7 +39,7 @@ func (vfsOS) Delete(path string, syncDir bool) error {
 	if err != nil {
 		return err
 	}
-	if canSyncDirs && syncDir {
+	if isUnix && syncDir {
 		f, err := os.Open(filepath.Dir(path))
 		if err != nil {
 			return _OK
@@ -96,7 +95,7 @@ func (vfsOS) OpenFilename(name *Filename, flags OpenFlag) (File, OpenFlag, error
 	if name == nil {
 		f, err = os.CreateTemp(os.Getenv("SQLITE_TMPDIR"), "*.db")
 	} else {
-		f, err = osutil.OpenFile(name.String(), oflags, 0666)
+		f, err = os.OpenFile(name.String(), oflags, 0666)
 	}
 	if err != nil {
 		if name == nil {
@@ -118,7 +117,7 @@ func (vfsOS) OpenFilename(name *Filename, flags OpenFlag) (File, OpenFlag, error
 			return nil, flags, _IOERR_FSTAT
 		}
 	}
-	if flags&OPEN_DELETEONCLOSE != 0 {
+	if isUnix && flags&OPEN_DELETEONCLOSE != 0 {
 		os.Remove(f.Name())
 	}
 
@@ -127,7 +126,8 @@ func (vfsOS) OpenFilename(name *Filename, flags OpenFlag) (File, OpenFlag, error
 		psow:     true,
 		atomic:   osBatchAtomic(f),
 		readOnly: flags&OPEN_READONLY != 0,
-		syncDir:  canSyncDirs && isCreate && isJournl,
+		syncDir:  isUnix && isCreate && isJournl,
+		delete:   !isUnix && flags&OPEN_DELETEONCLOSE != 0,
 		shm:      NewSharedMemory(name.String()+"-shm", flags),
 	}
 	return &file, flags, nil
@@ -141,6 +141,7 @@ type vfsFile struct {
 	keepWAL  bool
 	syncDir  bool
 	atomic   bool
+	delete   bool
 	psow     bool
 }
 
@@ -154,6 +155,9 @@ var (
 )
 
 func (f *vfsFile) Close() error {
+	if f.delete {
+		defer os.Remove(f.Name())
+	}
 	if f.shm != nil {
 		f.shm.Close()
 	}
@@ -177,7 +181,7 @@ func (f *vfsFile) Sync(flags SyncFlag) error {
 	if err != nil {
 		return err
 	}
-	if canSyncDirs && f.syncDir {
+	if isUnix && f.syncDir {
 		f.syncDir = false
 		d, err := os.Open(filepath.Dir(f.File.Name()))
 		if err != nil {
@@ -208,6 +212,9 @@ func (f *vfsFile) DeviceCharacteristics() DeviceCharacteristic {
 	if f.psow {
 		ret |= IOCAP_POWERSAFE_OVERWRITE
 	}
+	if runtime.GOOS == "windows" {
+		ret |= IOCAP_UNDELETABLE_WHEN_OPEN
+	}
 	return ret
 }
 
@@ -216,6 +223,9 @@ func (f *vfsFile) SizeHint(size int64) error {
 }
 
 func (f *vfsFile) HasMoved() (bool, error) {
+	if runtime.GOOS == "windows" {
+		return false, nil
+	}
 	fi, err := f.Stat()
 	if err != nil {
 		return false, err

vfs/os_std.go

@@ -8,8 +8,8 @@ import (
 )
 
 const (
+	isUnix      = false
 	_O_NOFOLLOW = 0
-	canSyncDirs = false
 )
 
 func osAccess(path string, flags AccessFlag) error {

vfs/os_unix.go

@@ -10,8 +10,8 @@ import (
 )
 
 const (
+	isUnix      = true
 	_O_NOFOLLOW = unix.O_NOFOLLOW
-	canSyncDirs = true
 )
 
 func osAccess(path string, flags AccessFlag) error {

vfs/os_windows.go

@@ -179,14 +179,13 @@ func osLockExTimeout(file *os.File, flags, start, len uint32, timeout time.Durat
 	if err != windows.ERROR_IO_PENDING {
 		return err
 	}
+	defer windows.CancelIoEx(fd, overlapped)
 
 	ms := (timeout + time.Millisecond - 1) / time.Millisecond
 	rc, err := windows.WaitForSingleObject(event, uint32(ms))
 	if rc == windows.WAIT_OBJECT_0 {
 		return nil
 	}
-	defer windows.CancelIoEx(fd, overlapped)
-
 	if err != nil {
 		return err
 	}

vfs/shm_windows.go

@@ -7,14 +7,12 @@ import (
 	"io"
 	"os"
 	"sync"
-	"syscall"
 	"time"
 
 	"github.com/tetratelabs/wazero/api"
 	"golang.org/x/sys/windows"
 
 	"github.com/ncruces/go-sqlite3/internal/util"
-	"github.com/ncruces/go-sqlite3/util/osutil"
 )
 
 type vfsShm struct {
@@ -33,7 +31,7 @@ type vfsShm struct {
 	sync.Mutex
 }
 
-// var _ blockingSharedMemory = &vfsShm{}
+var _ blockingSharedMemory = &vfsShm{}
 
 func (s *vfsShm) Close() error {
 	// Unmap regions.
@@ -48,12 +46,19 @@ func (s *vfsShm) Close() error {
 
 func (s *vfsShm) shmOpen() _ErrorCode {
 	if s.File == nil {
-		f, err := osutil.OpenFile(s.path,
+		path, err := windows.UTF16PtrFromString(s.path)
-			os.O_RDWR|os.O_CREATE|syscall.O_NONBLOCK, 0666)
 		if err != nil {
 			return _CANTOPEN
 		}
-		s.File = f
+		h, err := windows.CreateFile(path,
+			windows.GENERIC_READ|windows.GENERIC_WRITE,
+			windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE,
+			nil, windows.OPEN_ALWAYS,
+			windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_OVERLAPPED, 0)
+		if err != nil {
+			return _CANTOPEN
+		}
+		s.File = os.NewFile(uintptr(h), s.path)
 	}
 	if s.fileLock {
 		return _OK
@@ -185,6 +190,6 @@ func (s *vfsShm) shmUnmap(delete bool) {
 	}
 }
 
-// func (s *vfsShm) shmEnableBlocking(block bool) {
+func (s *vfsShm) shmEnableBlocking(block bool) {
-// 	s.blocking = block
+	s.blocking = block
-// }
+}