## Summary In Go applications using `modernc.org/sqlite` on Linux, standard POSIX advisory locking (`F_SETLK` / `F_SETLKW` / `F_GETLK`) presents a subtle data-safety hazard: **POSIX locks are process-scoped and tied to `(PID, inode)`**. If any goroutine or third-party library opens and subsequently closes an independent `os.File` descriptor pointing to the same database inode (e.g., for backup checks, hashing, or metadata inspection), calling `os.Close()` unconditionally instructs the Linux kernel to drop **all** POSIX locks held by the process on that inode. This strips the active SQLite transaction lock out from under a running connection, risking database corruption. We propose adding support for Linux **Open File Description (OFD) locks** (`F_OFD_SETLK = 37`, `F_OFD_SETLKW = 38`, `F_OFD_GETLK = 36`) with an automatic one-time fallback to standard POSIX locks if the kernel or filesystem returns `EINVAL`. --- ## Technical Design & Findings ### 1. The C Wrapper (`osFcntlOfd`) In `libsqlite3`, we can intercept locking calls in `os_unix.c` / `sqlite3.c` by wrapping `osFcntl`: ```c #if defined(F_OFD_SETLK) static volatile int ofdSupported = 1; static int osFcntlOfd(int fd, int cmd, void *arg){ if( ofdSupported ){ int ofdCmd = cmd; if( cmd==F_SETLK ) ofdCmd = F_OFD_SETLK; else if( cmd==F_SETLKW ) ofdCmd = F_OFD_SETLKW; else if( cmd==F_GETLK ) ofdCmd = F_OFD_GETLK; if( ofdCmd!=cmd ){ struct flock *pLock = (struct flock *)arg; pLock->l_pid = 0; /* Required by Linux kernel for F_OFD_* commands */ int rc = osFcntl(fd, ofdCmd, arg); if( rc!=(-1) || errno!=EINVAL ){ return rc; } ofdSupported = 0; } } return osFcntl(fd, cmd, arg); } #undef osFcntl #define osFcntl osFcntlOfd #endif ``` ### 2. Critical Kernel Requirement: `flock.l_pid == 0` During testing, we discovered why naive OFD locking fails in SQLite: the Linux kernel strictly requires `flock.l_pid == 0` when setting OFD locks (`F_OFD_SETLK` / `F_OFD_SETLKW`). Because SQLite C code leaves `lock.l_pid` uninitialized on stack allocations when preparing advisory locks, the kernel rejects the call with `EINVAL`. Explicitly setting `pLock->l_pid = 0` before invoking `F_OFD_*` resolves this. Note that zeroing `pLock->l_pid = 0` without saving or restoring its prior value is completely safe across both OFD and standard POSIX locks: * For `F_OFD_SETLK` / `F_OFD_SETLKW`, the Linux kernel requires `l_pid == 0`. * For POSIX `F_SETLK` / `F_SETLKW` (if falling back), POSIX defines `l_pid` as ignored on input when acquiring or releasing locks. * For `F_GETLK` / `F_OFD_GETLK`, `l_pid` is an output field populated by the kernel if a conflicting lock is found. * Additionally, `struct flock` in SQLite is always allocated as a function-local stack variable (e.g. in `unixFileLock`), so there are no cross-thread concurrency hazards. ### 3. Automatic One-Time Fallback By declaring `ofdSupported` as `static volatile int` (`ccgo` translates this to atomic memory loads/stores), if an older kernel or network filesystem rejects `F_OFD_*` with `EINVAL`, `ofdSupported` permanently flips to `0` and all subsequent locking calls execute standard POSIX `F_SETLK` with zero overhead. --- ## Verification We validated this implementation across `libc`, `libsqlite3`, and `sqlite` with a unit test (`TestOFDLockSurvivesOSClose`): 1. Open a SQLite connection and begin an exclusive write transaction. 2. Open and close an independent `os.File` descriptor to the exact same database file path. 3. Attempt to acquire an OS lock from a probe descriptor: * Under legacy POSIX locks (`F_SETLK`), `os.Close()` strips the lock and the probe lock succeeds (unsafe). * Under OFD locks (`F_OFD_SETLK`), probing the lock returns `EAGAIN` / `EACCES`, confirming that **the database lock persists across `os.Close()`**. --- ## Open Design Question for Maintainers: Default vs. Opt-In DSN Parameter There is an important behavioral difference between POSIX locks and OFD locks when using connection pools: * **POSIX locks (`F_SETLK`)**: Because locks are owned by PID, multiple file descriptors opened by `database/sql` in the same process do not conflict at the kernel level. * **OFD locks (`F_OFD_SETLK`)**: Locks are owned per open file description. Consequently, concurrent queries across a connection pool without a configured busy timeout will experience OS-level contention (`SQLITE_BUSY`), which causes tests like `TestIssue20` and `TestIssue65` to fail unless a busy timeout is set. We would appreciate your guidance on which approach you prefer: 1. **Make OFD locking the default on Linux** (with automatic fallback to POSIX on `EINVAL`), updating pool/test expectations where appropriate; OR 2. **Gate OFD locking behind an opt-in DSN parameter** (e.g., `_ofd_lock=1`) so existing users relying on intra-process POSIX lock sharing retain the current default behavior. --- ## Associated Merge Requests (Source-Only Diffs) To respect repository separation and keep diffs easy to review, we have submitted **source-only Draft Merge Requests (excluding generated `ccgo_*.go` and vendored `lib/sqlite_*.go` files)** in bottom-up dependency order: ### Core OFD Locking Implementation 1. **[`cznic/libc!33`](https://gitlab.com/cznic/libc/-/merge_requests/33)**: Export Linux OFD lock constants (`F_OFD_GETLK`, `F_OFD_SETLK`, `F_OFD_SETLKW`) across `fcntl_linux_*.go` and add OFD switch routing to `internal/overlay/musl/src/fcntl/fcntl.c`. 2. **[`cznic/libsqlite3!3`](https://gitlab.com/cznic/libsqlite3/-/merge_requests/3)** *(requires libc release bump)*: Add `internal/sqlite_issue255.patch` (amalgamation `sqlite3.c`) and `internal/sqlite_issue255.patch2` (full source tree `src/os_unix.c`) and patch invocation steps in `generator.go`. 3. **[`cznic/sqlite!136`](https://gitlab.com/cznic/sqlite/-/merge_requests/136)** *(requires libsqlite3 release bump)*: Add `ofd_linux_test.go` (`TestOFDLockSurvivesOSClose`) verifying lock survival across `os.Close()`. ### Companion Toolchain & Build Fixes During local verification and cross-compilation (`make generate` / `make build_all_targets`), we also prepared companion fixes needed to generate and link all targets cleanly on modern Linux and Windows toolchains: * **[`cznic/libc!32`](https://gitlab.com/cznic/libc/-/merge_requests/32)**: Implement and export Windows CRT stat variants (`_stat32i64`, `__stat32i64`, `__stat64i32`) referenced by SQLite's Windows VFS (`os_win.c`). * **[`cznic/libsqlite3!2`](https://gitlab.com/cznic/libsqlite3/-/merge_requests/2)**: Add C23 preprocessor macro stubs (`-D_GCC_NULLPTR_T`, `-D_Float16=short`, `-D__bf16=short`) to `ccgo` invocations in `generator.go` so modern glibc/GCC headers parse without AST errors.