OPFSAdaptiveVFS: release the open lock when an open fails

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 });

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 });
     });
   }
 });

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 });
+  }
+});

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');
+      });
+  });
+}