Fix deadlock when transaction starts during another transaction

src/processor.ts

@@ -73,6 +73,8 @@ export class SQLocalProcessor {
 					`The origin private file system is not available, so ${databasePath} will not be persisted. Make sure your web server is configured to use the correct HTTP response headers (See https://sqlocal.dallashoffman.com/guide/setup#cross-origin-isolation).`
 				);
 			}
+
+			this.userFunctions.forEach(this.initUserFunction);
 		} catch (error) {
 			this.emitMessage({
 				type: 'error',
@@ -81,11 +83,9 @@ export class SQLocalProcessor {
 			});
 
 			this.destroy();
-			return;
+		} finally {
+			await this.initMutex.unlock();
 		}
-
-		this.userFunctions.forEach(this.initUserFunction);
-		await this.initMutex.unlock();
 	};
 
 	postMessage = async (
@@ -142,17 +142,6 @@ export class SQLocalProcessor {
 	): Promise<void> => {
 		if (!this.db) return;
 
-		const partOfTransaction =
-			(message.type === 'transaction' &&
-				(this.transactionKey === null ||
-					message.transactionKey === this.transactionKey)) ||
-			(message.type === 'query' &&
-				message.transactionKey === this.transactionKey);
-
-		if (!partOfTransaction) {
-			await this.transactionMutex.lock();
-		}
-
 		try {
 			const response: DataMessage = {
 				type: 'data',
@@ -162,17 +151,35 @@ export class SQLocalProcessor {
 
 			switch (message.type) {
 				case 'query':
-					const statementData = execOnDb(this.db, message);
-					response.data.push(statementData);
+					const partOfTransaction =
+						this.transactionKey !== null &&
+						this.transactionKey === message.transactionKey;
+
+					try {
+						if (!partOfTransaction) {
+							await this.transactionMutex.lock();
+						}
+						const statementData = execOnDb(this.db, message);
+						response.data.push(statementData);
+					} finally {
+						if (!partOfTransaction) {
+							await this.transactionMutex.unlock();
+						}
+					}
 					break;
 
 				case 'batch':
-					this.db.transaction((tx: Sqlite3Db) => {
-						for (let statement of message.statements) {
-							const statementData = execOnDb(tx, statement);
-							response.data.push(statementData);
-						}
-					});
+					try {
+						await this.transactionMutex.lock();
+						this.db.transaction((tx: Sqlite3Db) => {
+							for (let statement of message.statements) {
+								const statementData = execOnDb(tx, statement);
+								response.data.push(statementData);
+							}
+						});
+					} finally {
+						await this.transactionMutex.unlock();
+					}
 					break;
 
 				case 'transaction':
@@ -182,7 +189,11 @@ export class SQLocalProcessor {
 						this.db.exec({ sql: 'BEGIN' });
 					}
 
-					if (message.action === 'commit' || message.action === 'rollback') {
+					if (
+						(message.action === 'commit' || message.action === 'rollback') &&
+						this.transactionKey !== null &&
+						this.transactionKey === message.transactionKey
+					) {
 						const sql = message.action === 'commit' ? 'COMMIT' : 'ROLLBACK';
 						this.db.exec({ sql });
 						this.transactionKey = null;
@@ -199,10 +210,6 @@ export class SQLocalProcessor {
 				queryKey: message.queryKey,
 			});
 		}
-
-		if (!partOfTransaction) {
-			await this.transactionMutex.unlock();
-		}
 	};
 
 	protected getDatabaseInfo = async (

test/get-database-file.test.ts

@@ -33,6 +33,6 @@ describe('getDatabaseFile', () => {
 
 	it('should not throw when requested database has not been created', async () => {
 		const { getDatabaseFile } = new SQLocal('blank.sqlite3');
-		expect(getDatabaseFile()).resolves.not.toThrow();
+		await expect(getDatabaseFile()).resolves.not.toThrow();
 	});
 });

test/init.test.ts

@@ -39,7 +39,7 @@ describe('init', () => {
 		const write = async () => {
 			await sql`INSERT INTO nums (num) VALUES (1)`;
 		};
-		expect(write).rejects.toThrowError(
+		await expect(write).rejects.toThrowError(
 			'SQLITE_IOERR_WRITE: sqlite3 result code 778: disk I/O error'
 		);
 

test/overwrite-database-file.test.ts

@@ -22,8 +22,8 @@ describe('overwriteDatabaseFile', async () => {
 		await db1.overwriteDatabaseFile(db2File);
 
 		const letters = db1.sql`SELECT * FROM letters`;
-		expect(letters).rejects.toThrow();
+		await expect(letters).rejects.toThrow();
 		const nums = db1.sql`SELECT * FROM nums`;
-		expect(nums).resolves.toEqual([{ num: 1 }, { num: 2 }, { num: 3 }]);
+		await expect(nums).resolves.toEqual([{ num: 1 }, { num: 2 }, { num: 3 }]);
 	});
 });

test/sql.test.ts

@@ -33,7 +33,7 @@ describe('sql', () => {
 
 		const multiSelect2 = async () =>
 			await sql`SELECT * FROM groceries WHERE id = ${3}; SELECT * FROM groceries WHERE id = ${2};`;
-		expect(multiSelect2).rejects.toThrow();
+		await expect(multiSelect2).rejects.toThrow();
 
 		const delete1 = await sql`DELETE FROM groceries WHERE id = 2 RETURNING *`;
 		expect(delete1).toEqual([{ id: 2, name: 'milk' }]);

test/transaction.test.ts

@@ -3,7 +3,7 @@ import { SQLocal } from '../src/index.js';
 import { sleep } from './test-utils/sleep.js';
 
 describe('transaction', () => {
-	const { sql, transaction } = new SQLocal('transaction-test.sqlite3');
+	const { sql, batch, transaction } = new SQLocal('transaction-test.sqlite3');
 
 	beforeEach(async () => {
 		await sql`CREATE TABLE groceries (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT NOT NULL)`;
@@ -50,7 +50,7 @@ describe('transaction', () => {
 		expect(selectData.length).toBe(0);
 	});
 
-	it('should isolate transaction mutations', async () => {
+	it('should isolate transaction mutations from outside queries', async () => {
 		const order: number[] = [];
 
 		await Promise.all([
@@ -73,4 +73,50 @@ describe('transaction', () => {
 		expect(data).toEqual([{ name: 'x' }, { name: 'x' }]);
 		expect(order).toEqual([1, 2, 3]);
 	});
+
+	it('should isolate transaction mutations from outside batch queries', async () => {
+		const order: number[] = [];
+
+		await Promise.all([
+			transaction(async (tx) => {
+				order.push(1);
+				await tx.sql`INSERT INTO groceries (name) VALUES ('a')`;
+				await sleep(200);
+				order.push(3);
+				await tx.sql`INSERT INTO groceries (name) VALUES ('b')`;
+			}),
+			(async () => {
+				await sleep(100);
+				order.push(2);
+				await batch((sql) => [sql`UPDATE groceries SET name = 'x'`]);
+			})(),
+		]);
+
+		const data = await sql`SELECT name FROM groceries`;
+
+		expect(data).toEqual([{ name: 'x' }, { name: 'x' }]);
+		expect(order).toEqual([1, 2, 3]);
+	});
+
+	it('should complete concurrent transactions', async () => {
+		const transactions = Promise.all([
+			transaction(async (tx) => {
+				await tx.sql`INSERT INTO groceries (name) VALUES ('a') RETURNING name`;
+				await sleep(100);
+				return await tx.sql`SELECT name FROM groceries`;
+			}),
+			transaction(async (tx) => {
+				await sleep(50);
+				return await tx.sql`INSERT INTO groceries (name) VALUES ('b') RETURNING name`;
+			}),
+		]);
+
+		await expect(transactions).resolves.toEqual([
+			[{ name: 'a' }],
+			[{ name: 'b' }],
+		]);
+
+		const data = await sql`SELECT name FROM groceries`;
+		expect(data).toEqual([{ name: 'a' }, { name: 'b' }]);
+	});
 });

vite.config.ts

@@ -3,6 +3,9 @@ import { defineConfig } from 'vite';
 
 export default defineConfig({
 	test: {
+		testTimeout: 1000,
+		hookTimeout: 1000,
+		teardownTimeout: 1000,
 		includeTaskLocation: true,
 		browser: {
 			enabled: true,