Good afternoon, team,

A third-party user recently found a bug in the "sahpool" OPFS VFS (that's the 2nd OPFS VFS) which doesn't actually break anything but leaves me with a bit of a conundrum of whether to actually fix it or leave it in its broken/no-op state...

Relevant links:

• /timeline?r=sahpool-digest
• which was prompted by: https://github.com/sqlite/sqlite-wasm/issues/97

Background

When OPFS opens a file, it's exclusively locked. To provide concurrency, the first OPFS VFS works around that by opening and closing the handles on demand, which works reasonably well but has a high performance hit. The second OPFS VFS ("sahpool", where SAH = SyncAccessHandle) takes a different approach, opening some fixed number of handles at startup and keeping them open, which is extremely performant but eliminates all concurrency options.

One of the things the sahpool does is set aside N "slots" for flies, each one assigned a random name, all of which live in a single directory. Each of those slots has a 4kb header for its metadata, which is followed by any data written via the VFS API. One of those pieces of metadata is the file name as the user defined it. Along with that is a digest/hash of just that name. If, when opening its files at init-time, the VFS sees that the digest does not match expectations, it assumes the file is corrupt and wipes/reuses that slot.

Sidebar: the concept of this digest, and the behavior assigned to it, were ported over from Roy Hashimoto's original implementation, but i used a simpler/naive hash which turned out to backfire because of JS's integer overflow rules.

That's where the bug comes in...

The digest algorithm was broken (100% my fault), in that it always ends up overflowing to Infinity, which then resolves to 0 when it's converted to an integer and saved alongside the file name in the metadata header. Ergo, sahpool file names always have a digest of 0, whether they're corrupt or not.

And this fix is...?

There are at least two ways to fix, both of which are currently code-ready:

1. We remove this digest altogether, as it's had no effect since that VFS's initial release some 18 months ago, aside from burning CPU cycles. We're unaware of any corruption related to this, but it's unlikely that we would be aware of it. The time-frame for corruption of a file's name (the only thing this digest accounts for) is astronomically small.

2. We fix the digest in a way which keeps older content working when a user upgrades to 3.50. If, however, a client upgrades to version 3.50, creates files, then downgrades to 3.4x, those files would be seen as corrupt and their file slots would be automatically cleared and reused. The odds of someone actually downgrading their sqlite3.wasm installation seem spectacularly low in the context of browser-side applications. Similarly, the odds of having corruption hit only in the filename segment of a file's metadata (the only part the digest covers) seem mind-bogglingly small.

Both of those approaches are checked in and ready to go, but that's my conundrum: do we (1) fix this by removing it altogether or do we (2) fix it properly but introduce a very slight chance for backwards incompatibility (and OPFS-side file loss) if someone upgrades, then downgrades, their sqlite3.wasm?

:-?

Sidebar: long-term persistence of OPFS-hosted files is always unreliable because they can be deleted at a browser's whim at any time. That argues for option (2). Option (1) has a certain appeal, though.

Edit: per /chat discussion, we'll be using option (2).