diff --git a/src/examples/OPFSAdaptiveVFS.js b/src/examples/OPFSAdaptiveVFS.js index 7deaa2e0..07405d7e 100644 --- a/src/examples/OPFSAdaptiveVFS.js +++ b/src/examples/OPFSAdaptiveVFS.js @@ -124,6 +124,13 @@ export class OPFSAdaptiveVFS extends WebLocksMixin(FacadeVFS) { return VFS.SQLITE_OK; } catch (e) { this.lastError = e; + + // Release at once what this open acquired, rather than leave it to + // xClose. + const file = this.mapIdToFile.get(fileId); + file?.openLockReleaser?.(); + file?.handleRequestChannel?.close(); + this.mapIdToFile.delete(fileId); return VFS.SQLITE_CANTOPEN; } } @@ -184,6 +191,11 @@ export class OPFSAdaptiveVFS extends WebLocksMixin(FacadeVFS) { this.mapIdToFile.delete(fileId); await file?.accessHandle?.close(); + // Release what the open and the last transaction may still hold. + file?.openLockReleaser?.(); + file?.handleLockReleaser?.(); + file?.handleRequestChannel?.close(); + if (file?.flags & VFS.SQLITE_OPEN_DELETEONCLOSE) { const [directoryHandle, name] = await getPathComponents(file.pathname, false); await directoryHandle.removeEntry(name, { recursive: false }); diff --git a/test/OPFSAdaptiveVFS.test.js b/test/OPFSAdaptiveVFS.test.js index 0a2861c8..aadaea28 100644 --- a/test/OPFSAdaptiveVFS.test.js +++ b/test/OPFSAdaptiveVFS.test.js @@ -4,6 +4,7 @@ import { vfs_xAccess } from "./vfs_xAccess.js"; import { vfs_xClose } from "./vfs_xClose.js"; import { vfs_xRead } from "./vfs_xRead.js"; import { vfs_xWrite } from "./vfs_xWrite.js"; +import { vfs_open_lock_recovery } from "./vfs_open_lock_recovery.js"; const CONFIG = 'OPFSAdaptiveVFS'; const BUILDS = ['asyncify', 'jspi']; @@ -22,6 +23,7 @@ describe(CONFIG, function() { vfs_xClose(context); vfs_xRead(context); vfs_xWrite(context); + vfs_open_lock_recovery({ build }); }); } }); diff --git a/test/vfs_open_lock_recovery-worker.js b/test/vfs_open_lock_recovery-worker.js new file mode 100644 index 00000000..20cc27cb --- /dev/null +++ b/test/vfs_open_lock_recovery-worker.js @@ -0,0 +1,36 @@ +// A worker for vfs_open_lock_recovery.js, holding one connection. Where the +// engine offers readwrite-unsafe access handles, OPFSAdaptiveVFS takes no +// lock on open; the test removes that feature first, so that the path taken +// is the one of engines without it. +import * as SQLite from '../src/sqlite-api.js'; + +const BUILDS = new Map([ + ['asyncify', '../dist/wa-sqlite-async.mjs'], + ['jspi', '../dist/wa-sqlite-jspi.mjs'], +]); + +const searchParams = new URLSearchParams(location.search); +const ready = (async () => { + if (searchParams.get('unsafe') === 'off') { + delete globalThis.FileSystemSyncAccessHandle.prototype.mode; + } + const { default: moduleFactory } = await import(BUILDS.get(searchParams.get('build'))); + const { OPFSAdaptiveVFS } = await import('../src/examples/OPFSAdaptiveVFS.js'); + const module = await moduleFactory(); + const sqlite3 = SQLite.Factory(module); + const vfs = await OPFSAdaptiveVFS.create('lock-recovery', module); + sqlite3.vfs_register(vfs, true); + return sqlite3; +})(); + +addEventListener('message', async ({ data }) => { + const sqlite3 = await ready; + try { + const db = await sqlite3.open_v2(data.filename); + await sqlite3.exec(db, 'SELECT 1'); + await sqlite3.close(db); + postMessage({ ok: true }); + } catch (e) { + postMessage({ ok: false, error: e.message }); + } +}); diff --git a/test/vfs_open_lock_recovery.js b/test/vfs_open_lock_recovery.js new file mode 100644 index 00000000..991932e3 --- /dev/null +++ b/test/vfs_open_lock_recovery.js @@ -0,0 +1,64 @@ +import { createHolder } from "./vfs_handle_recovery.js"; + +const OPEN_BOUND_MS = 5000; + +/** + * Recovery after an open whose access handle could not be acquired, on the + * path OPFSAdaptiveVFS takes without readwrite-unsafe access handles: there + * it takes the file's lock first, then the handle. When the handle fails, + * the lock must be released with it. + * @param {{ build: string }} params + */ +export function vfs_open_lock_recovery({ build }) { + describe('vfs_open_lock_recovery', function() { + const cleanup = []; + afterEach(async function() { + for (const fn of cleanup.splice(0).reverse()) { + await fn(); + } + }); + + function connect() { + const url = new URL('./vfs_open_lock_recovery-worker.js', import.meta.url); + url.searchParams.set('build', build); + url.searchParams.set('unsafe', 'off'); + const worker = new Worker(url, { type: 'module' }); + cleanup.push(() => worker.terminate()); + return (filename) => new Promise((resolve, reject) => { + const bound = setTimeout(() => resolve({ ok: false, error: 'hung' }), OPEN_BOUND_MS); + worker.addEventListener('message', ({ data }) => { + clearTimeout(bound); + resolve(data); + }, { once: true }); + worker.addEventListener('error', event => reject(new Error(event.message)), { once: true }); + worker.postMessage({ filename }); + }); + } + + it('should open a database once the file it could not acquire is released', + async function() { + const filename = `lock-recovery-${Math.random().toString(36).slice(2)}`; + const root = await navigator.storage.getDirectory(); + cleanup.push(() => root.removeEntry(filename).catch(() => {})); + + const holder = createHolder(); + cleanup.push(() => holder.dispose()); + const taken = await holder.take(filename); + if (!taken.ok) { + pending(`cannot hold an exclusive handle: ${taken.error}`); + return; + } + + // Expected to fail: the file is held elsewhere. + const open = connect(); + expect((await open(filename)).ok).toBeFalse(); + + await holder.release(); + + // The file is free, so this must succeed. When the failed open kept + // the file's lock, this one waited for it forever. + const opened = await open(filename); + expect(opened.ok ? 'opened' : opened.error).toBe('opened'); + }); + }); +}