iVersion: 3 with xFetch: None causes SEGFAULT under concurrent WAL

[russellromney]

Bug

sqlite3_io_methods.iVersion is set to 3 in register_inner() (src/vfs.rs:316), but xFetch and xUnfetch are set to None (null function pointers).

SQLite interprets iVersion >= 3 as "xFetch and xUnfetch are implemented and callable." It does not null-check these function pointers before calling them. When SQLite's pager decides to memory-map a page (which happens during WAL checkpoint page reads under concurrent load), it calls xFetch through the null pointer, causing a SEGFAULT.

Reproduction

Any VFS using sqlite-plugin with WAL mode under concurrent readers + writers. The crash is intermittent because SQLite's mmap heuristic doesn't always choose to mmap pages.

Minimal reproduction: 4 reader threads + 2 writer threads, WAL mode, 2000 rows of 5KB each, 3 seconds. ~66% crash rate on macOS aarch64.

ASAN output:

==59391==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
(pc 0x000000000000 bp ... sp ... T8)
==59391==Hint: pc points to the zero page.

    #0 0x000000000000  (<unknown module>)
    #1 ... in sqlite3WalCheckpoint
    #2 ... in sqlite3PagerCheckpoint
    #3 ... in sqlite3BtreeCheckpoint
    ...

pc=0x0 confirms a null function pointer call (not a null data pointer dereference).

Fix

Set iVersion: 2 instead of iVersion: 3. Version 2 declares SHM/WAL support (xShmMap, xShmLock, xShmBarrier, xShmUnmap) without claiming xFetch/xUnfetch support.

let io_methods = ffi::sqlite3_io_methods {
    iVersion: 2,  // was 3
    // ...
    xFetch: None,
    xUnfetch: None,
};

The one-line change makes the crash go away completely (30/30 concurrent stress test passes, was ~10/30 before).

Context

From the SQLite header (sqlite3.h):

struct sqlite3_io_methods {
  int iVersion;
  // ... version 1 methods ...
  /* Methods above are valid for version 1 */
  int (*xShmMap)(...);
  int (*xShmLock)(...);
  void (*xShmBarrier)(...);
  int (*xShmUnmap)(...);
  /* Methods above are valid for version 2 */
  int (*xFetch)(...);
  int (*xUnfetch)(...);
  /* Methods above are valid for version 3 */
};

"Valid for version N" means "guaranteed non-null and callable when iVersion >= N." Setting iVersion=3 with null xFetch violates this contract.

Longer term

Once sqlite-plugin implements actual xFetch/xUnfetch (memory-mapped page reads), iVersion can be bumped back to 3. This would enable zero-copy page reads for VFS implementations that support it, which is a significant performance win for warm-cache workloads.

Found while building turbolite.

[russellromney]

Root cause found

The bug was introduced in PR #76 ("Share memory support"). That PR added xShmMap/xShmLock/xShmBarrier/xShmUnmap bindings and bumped iVersion from 1 to 3. But it should have bumped to 2, not 3.

The SQLite iVersion contract:

• 1 = basic file I/O (xClose through xDeviceCharacteristics)
• 2 = + SHM/WAL support (xShmMap, xShmLock, xShmBarrier, xShmUnmap)
• 3 = + memory-mapped I/O (xFetch, xUnfetch)

PR #76 left xFetch: None and xUnfetch: None but declared iVersion=3, which tells SQLite "xFetch is implemented and callable." SQLite trusts iVersion and does not null-check function pointers before calling them.

Reproduction (no external VFS needed)

I wrote a standalone regression test using a minimal file-backed VFS (no turbolite, no external dependencies beyond libc). It SEGFAULTs ~60% of the time on unpatched main with 1 writer + 4 readers in WAL mode for 3 seconds.

ASAN confirms: pc=0x000000000000 (null function pointer call) inside sqlite3WalCheckpoint -> sqlite3OsFetch.

Fix

I have implemented proper fetch() and unfetch() methods on the Vfs trait with safe defaults:

• fetch() returns Ok(None) by default, telling SQLite "I can't mmap this page, use xRead instead." SQLite gracefully falls back.
• unfetch() is a no-op by default.
• Both are wired into io_methods as x_fetch/x_unfetch, making iVersion: 3 legitimate.
• VFS implementations that want actual mmap can override fetch() to return NonNull pointers.

This keeps iVersion=3, adds the missing trait methods, and maintains backward compatibility (default implementations decline mmap).

Branch with fix + regression test: russellromney/sqlite-plugin:fix/concurrent-wal-segfault

30/30 passes on the regression test after the fix.

[carlsverre]

Damn, this is a good catch. I remember feeling weird about the xFetch/xUnfetch stuff, but I should have done more research. It looks both correct and safe to roll back the version to 2. I don't plan on adding xFetch/xUnfetch anytime soon unless there is a valid proposed use case.

[russellromney]

Glad to be of help. Yeah after I found the bug and reproduced it I went looking at previous stuff and your "unsafe" comment caught my eye. I was so sure it was the turbolite vfs code! haha

I have a use case for xFetch/xUnfetch, turbolite uses a local NVMe cache file for page storage. Currently every cached page read goes through pread (~5us). With xFetch we could mmap the cache file and return a direct pointer, getting page reads down to pointer dereference latency (<1us). That's meaningful for warm workloads where we're serving thousands of point lookups per second from cache. turbolite is significantly slower for repeated point lookups in the multi-reader-while-writing scenario. So it would be awesome to kill that latency.

We've already implemented fetch()/unfetch() on the Vfs trait in our fork with safe defaults (fetch returns None to decline mmap, unfetch is a no-op). VFS implementations that don't want mmap get the same behavior as iVersion=2. I'll add the PR for your review, let's discuss more there

[carlsverre]

Turbolite is really, really cool, btw. Awesome to see someone lead the charge with SQLite backed by object storage. I'm a bit bummed I couldn't find a reasonable path forward with my attempt at this (graft.rs). Hopefully I'll revisit that project, and maybe learn something from Turbolite in the meantime :)

Will probably go with the fetch/unfetch PR. reviewing now.

[russellromney]

Thanks, that means a lot, i appreciated your hacker news comments, and sqlite-plugin's direct pointer model for shm_map was actually one of the reasons I migrated from sqlite-vfs. turbolite's come a long way from there even, especially in hardening/reliability. And i've integrated https://github.com/russellromney/haqlite with turbolite now and am pretty excited about that direction. Much to look forward to all around