Skip to content

Commit

Permalink
Merge pull request #226 from powersync-ja/fix-web-locks
Browse files Browse the repository at this point in the history
Fix web lock issues
  • Loading branch information
rkistner authored Jul 16, 2024
2 parents 2140b80 + a1b52be commit 9f35b78
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/real-bottles-mate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@powersync/web': patch
---

Fix read statements not using the transaction lock
2 changes: 0 additions & 2 deletions packages/kysely-driver/tests/setup/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,3 @@ export const getPowerSyncDb = () => {

return database;
};

export const getKyselyDb = wrapPowerSyncWithKysely<Database>(getPowerSyncDb());
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ describe('PowerSyncConnection', () => {

it('should execute a select query using getAll from the table', async () => {
await powerSyncDb.execute('INSERT INTO users (id, name) VALUES(uuid(), ?)', ['John']);

const getAllSpy = vi.spyOn(powerSyncDb, 'getAll');

const compiledQuery: CompiledQuery = {
Expand All @@ -35,7 +34,7 @@ describe('PowerSyncConnection', () => {

expect(rows.length).toEqual(1);
expect(rows[0].name).toEqual('John');
expect(getAllSpy).toHaveBeenCalledTimes(1);
expect(getAllSpy).toHaveBeenCalledWith('SELECT * From users', []);
});

it('should insert to the table', async () => {
Expand Down
4 changes: 3 additions & 1 deletion packages/web/src/db/adapters/wa-sqlite/WASQLiteDBAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ export class WASQLiteDBAdapter extends BaseObserver<DBAdapterListener> implement
this.dbGetHelpers = null;
this.methods = null;
this.initialized = this.init();
this.dbGetHelpers = this.generateDBHelpers({ execute: this._execute.bind(this) });
this.dbGetHelpers = this.generateDBHelpers({
execute: (query, params) => this.acquireLock(() => this._execute(query, params))
});
}

get name() {
Expand Down
19 changes: 12 additions & 7 deletions packages/web/src/shared/open-db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as SQLite from '@journeyapps/wa-sqlite';
import '@journeyapps/wa-sqlite';
import * as Comlink from 'comlink';
import type { DBFunctionsInterface, OnTableChangeCallback, WASQLExecuteResult } from './types';
import { Mutex } from 'async-mutex';

let nextId = 1;

Expand All @@ -18,6 +19,7 @@ export async function _openDB(
sqlite3.vfs_register(vfs, true);

const db = await sqlite3.open_v2(dbFileName);
const statementMutex = new Mutex();

/**
* Listeners are exclusive to the DB connection.
Expand All @@ -40,10 +42,10 @@ export async function _openDB(

/**
* This requests a lock for executing statements.
* Should only be used interanlly.
* Should only be used internally.
*/
const _acquireExecuteLock = (callback: () => Promise<any>): Promise<any> => {
return navigator.locks.request(`db-execute-${dbFileName}`, callback);
const _acquireExecuteLock = <T>(callback: () => Promise<T>): Promise<T> => {
return statementMutex.runExclusive(callback);
};

/**
Expand Down Expand Up @@ -115,7 +117,7 @@ export async function _openDB(
* This executes SQL statements in a batch.
*/
const executeBatch = async (sql: string, bindings?: any[][]): Promise<WASQLExecuteResult> => {
return _acquireExecuteLock(async () => {
return _acquireExecuteLock(async (): Promise<WASQLExecuteResult> => {
let affectedRows = 0;

const str = sqlite3.str_new(db, sql);
Expand All @@ -127,7 +129,8 @@ export async function _openDB(
const prepared = await sqlite3.prepare_v2(db, query);
if (prepared === null) {
return {
rowsAffected: 0
rowsAffected: 0,
rows: { _array: [], length: 0 }
};
}
const wrappedBindings = bindings ? bindings : [];
Expand Down Expand Up @@ -158,13 +161,15 @@ export async function _openDB(
} catch (err) {
await executeSingleStatement('ROLLBACK');
return {
rowsAffected: 0
rowsAffected: 0,
rows: { _array: [], length: 0 }
};
} finally {
sqlite3.str_finish(str);
}
const result = {
rowsAffected: affectedRows
rowsAffected: affectedRows,
rows: { _array: [], length: 0 }
};

return result;
Expand Down
22 changes: 22 additions & 0 deletions packages/web/tests/crud.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { AbstractPowerSyncDatabase, Column, ColumnType, CrudEntry, Schema, Table
import { PowerSyncDatabase } from '@powersync/web';
import { v4 as uuid } from 'uuid';
import { generateTestDb } from './utils/testDb';
import pDefer from 'p-defer';

const testId = '2290de4f-0488-4e50-abed-f8e8eb1d0b42';

Expand Down Expand Up @@ -289,4 +290,25 @@ describe('CRUD Tests', () => {
await tx2.complete();
expect(await powersync.getNextCrudTransaction()).equals(null);
});

it('Transaction exclusivity', async () => {
const outside = pDefer();
const inTx = pDefer();

const txPromise = powersync.writeTransaction(async (tx) => {
await tx.execute('INSERT INTO assets(id, description) VALUES(?, ?)', [testId, 'test1']);
inTx.resolve();
await outside.promise;
await tx.rollback();
});

await inTx.promise;

const r = powersync.getOptional<any>('SELECT * FROM assets WHERE id = ?', [testId]);
await new Promise((resolve) => setTimeout(resolve, 10));
outside.resolve();

await txPromise;
expect(await r).toEqual(null);
});
});

0 comments on commit 9f35b78

Please sign in to comment.