diff --git a/README.md b/README.md index bfece94b..b5c7e208 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,10 @@ const db = open({ name: 'myDb.sqlite' }) - **Sync** (`execute`, `executeBatch`, `loadFile`): Run on the JS thread. Use for small, fast work; heavy work can block the UI. - **Async** (`executeAsync`, `executeBatchAsync`, `loadFileAsync`, `transaction`): Run off the JS thread. Prefer these for larger or many queries to keep the app responsive. +Async operations submitted on the opened `db` connection outside a transaction callback run in call order. Async work waits for an active transaction to finish, while a conflicting sync operation or `close()` throws a busy error. + +`NitroSQLite.native` bypasses this JavaScript queue. Native calls keep each individual SQLite handle safe, but mixing them with a session transaction can still run statements inside that transaction. A build with `SQLITE_THREADSAFE=0` also remains unsafe when different database handles run concurrently unless the caller serializes every SQLite call globally. + --- # Basic usage @@ -105,6 +109,8 @@ const users = db.execute<{ id: number; name: string }>( Use `db.transaction()` for multiple statements in a single transaction. The callback receives a `tx` object with `execute`, `executeAsync`, `commit`, and `rollback`. If the callback throws, the transaction is rolled back. Otherwise it is committed when the callback resolves (or you can call `tx.commit()` / `tx.rollback()` explicitly). +Inside the callback, all database work, including work in helper functions, must use the passed `tx` object. Do not await `db.executeAsync()`, `db.executeBatchAsync()`, or another queued session/global operation for the same database from inside the callback. Those operations wait for the transaction to finish, while the transaction would wait for them, creating a deadlock. Sync session/global calls for that database throw a busy error instead. + ```typescript await db.transaction(async (tx) => { tx.execute('UPDATE sometable SET somecolumn = ? WHERE somekey = ?', [0, 1]) diff --git a/example/tests/unit/specs/DatabaseQueue.spec.ts b/example/tests/unit/specs/DatabaseQueue.spec.ts index d80c5acb..e5822ce2 100644 --- a/example/tests/unit/specs/DatabaseQueue.spec.ts +++ b/example/tests/unit/specs/DatabaseQueue.spec.ts @@ -7,12 +7,39 @@ import { } from '@tests/unit/common' import { describe, it } from '@tests/TestApi' import { testDb, testDbQueue } from '@tests/db' -import type { BatchQueryCommand } from 'react-native-nitro-sqlite' +import { + NitroSQLite, + NitroSQLiteError, + open, + type BatchQueryCommand, +} from 'react-native-nitro-sqlite' const TEST_QUERY = 'SELECT * FROM [User];' const TEST_BATCH_COMMANDS: BatchQueryCommand[] = [{ query: TEST_QUERY }] +function createDeferred() { + let resolve!: () => void + const promise = new Promise((promiseResolve) => { + resolve = promiseResolve + }) + return { promise, resolve } +} + +function dropDatabaseIfExists(dbName: string, location?: string) { + try { + NitroSQLite.native.drop(dbName, location) + } catch (error) { + if ( + error instanceof Error && + error.message.includes('Database file not found') + ) { + return + } + throw error + } +} + export default function registerDatabaseQueueUnitTests() { describe('Database Queue', () => { it('multiple transactions are queued', async () => { @@ -178,5 +205,354 @@ export default function registerDatabaseQueueUnitTests() { } } }) + + it('queues ordinary async work behind a transaction rollback', async () => { + const transactionStarted = createDeferred() + const finishTransaction = createDeferred() + const transactionPromise = testDb.transaction(async (tx) => { + tx.execute( + 'INSERT INTO User (id, name, age, networth) VALUES (?, ?, ?, ?)', + [1, 'transaction', 1, 1], + ) + transactionStarted.resolve() + await finishTransaction.promise + throw new Error('rollback transaction') + }) + + await transactionStarted.promise + const externalWrite = testDb.executeAsync( + 'INSERT INTO User (id, name, age, networth) VALUES (?, ?, ?, ?)', + [2, 'external', 2, 2], + ) + finishTransaction.resolve() + + try { + await transactionPromise + } catch (error) { + expect((error as Error).message).toContain('rollback transaction') + } + await externalWrite + + expect( + testDb.execute<{ id: number }>('SELECT id FROM User').results, + ).toEqual([{ id: 2 }]) + }) + + it('returns distinct insert IDs from parallel async inserts', async () => { + testDb.execute( + 'CREATE TABLE ConcurrentInsert (id INTEGER PRIMARY KEY AUTOINCREMENT, value TEXT)', + ) + + const results = await Promise.all( + Array.from({ length: 24 }, (_, index) => + testDb.executeAsync( + 'INSERT INTO ConcurrentInsert (value) VALUES (?)', + [`value-${index}`], + ), + ), + ) + + expect(results.map((result) => result.insertId)).toEqual( + Array.from({ length: 24 }, (_, index) => index + 1), + ) + }) + + it('rejects sync work and close while async work is pending', async () => { + const dbName = 'busy-close' + dropDatabaseIfExists(dbName) + const db = open({ name: dbName }) + + const pending = db.executeAsync( + 'WITH RECURSIVE counter(value) AS (VALUES(0) UNION ALL SELECT value + 1 FROM counter WHERE value < 100000) SELECT sum(value) FROM counter', + ) + + const syncOperations = [ + () => db.execute('SELECT 1'), + () => db.executeBatch([{ query: 'SELECT 1' }]), + () => db.loadFile('/nitro-sqlite-does-not-exist.sql'), + () => db.attach('other.sqlite', 'other'), + () => db.detach('other'), + () => db.delete(), + () => db.close(), + ] + + for (const operation of syncOperations) { + let operationError: unknown + try { + operation() + } catch (error) { + operationError = error + } + + expect(operationError).toBeInstanceOf(NitroSQLiteError) + expect((operationError as Error).message).toContain('busy') + } + + await pending + db.close() + const reopened = open({ name: dbName }) + reopened.close() + reopened.delete() + }) + + it('releases the queue after loadFileAsync rejects', async () => { + let loadError: unknown + try { + await testDb.loadFileAsync('/nitro-sqlite-does-not-exist.sql') + } catch (error) { + loadError = error + } + + expect(loadError).toBeInstanceOf(NitroSQLiteError) + expect((loadError as Error).message).toContain('Could not load file') + expect((await testDb.executeAsync('SELECT 42 AS value')).results).toEqual( + [{ value: 42 }], + ) + }) + + it('does not block operations on another database', async () => { + const firstName = 'independent-first' + const secondName = 'independent-second' + dropDatabaseIfExists(firstName) + dropDatabaseIfExists(secondName) + const first = open({ name: firstName }) + const second = open({ name: secondName }) + const transactionStarted = createDeferred() + const finishTransaction = createDeferred() + + try { + const transactionPromise = first.transaction(async () => { + transactionStarted.resolve() + await finishTransaction.promise + }) + await transactionStarted.promise + + const result = await second.executeAsync('SELECT 42 AS value') + expect(result.results).toEqual([{ value: 42 }]) + + finishTransaction.resolve() + await transactionPromise + } finally { + finishTransaction.resolve() + first.close() + first.delete() + second.close() + second.delete() + } + }) + + it('rejects a duplicate session open without replacing the original connection', () => { + const dbName = 'duplicate-session-open' + dropDatabaseIfExists(dbName) + dropDatabaseIfExists(dbName, '..') + + const db = open({ name: dbName }) + + try { + db.execute('CREATE TABLE ConnectionMarker (value TEXT NOT NULL)') + db.execute('INSERT INTO ConnectionMarker (value) VALUES (?)', [ + 'original', + ]) + + let duplicateError: unknown + try { + open({ name: dbName, location: '..' }) + } catch (error) { + duplicateError = error + } + + expect(duplicateError).toBeInstanceOf(NitroSQLiteError) + expect((duplicateError as Error).message).toContain('already open') + expect( + db.execute<{ value: string }>('SELECT value FROM ConnectionMarker') + .results, + ).toEqual([{ value: 'original' }]) + } finally { + db.close() + dropDatabaseIfExists(dbName) + dropDatabaseIfExists(dbName, '..') + } + }) + + it('rejects duplicate direct native opens', () => { + const dbName = 'duplicate-native-open' + dropDatabaseIfExists(dbName) + + NitroSQLite.native.open(dbName) + + try { + NitroSQLite.execute( + dbName, + 'CREATE TABLE ConnectionMarker (value TEXT NOT NULL)', + ) + NitroSQLite.execute( + dbName, + 'INSERT INTO ConnectionMarker (value) VALUES (?)', + ['original'], + ) + + let duplicateError: unknown + try { + NitroSQLite.native.open(dbName) + } catch (error) { + duplicateError = error + } + + expect(duplicateError).toBeInstanceOf(Error) + expect((duplicateError as Error).message).toContain('already open') + expect( + NitroSQLite.execute<{ value: string }>( + dbName, + 'SELECT value FROM ConnectionMarker', + ).results, + ).toEqual([{ value: 'original' }]) + } finally { + NitroSQLite.native.close(dbName) + dropDatabaseIfExists(dbName) + } + }) + + it('preserves an open connection when deleting a missing target', () => { + const dbName = 'missing-delete-target' + dropDatabaseIfExists(dbName) + dropDatabaseIfExists(dbName, '..') + const db = open({ name: dbName }) + + try { + db.execute('CREATE TABLE ConnectionMarker (value TEXT NOT NULL)') + db.execute('INSERT INTO ConnectionMarker (value) VALUES (?)', [ + 'original', + ]) + + let deleteError: unknown + try { + NitroSQLite.native.drop(dbName, '..') + } catch (error) { + deleteError = error + } + + expect(deleteError).toBeInstanceOf(Error) + expect((deleteError as Error).message).toContain( + 'Database file not found', + ) + expect( + db.execute<{ value: string }>('SELECT value FROM ConnectionMarker') + .results, + ).toEqual([{ value: 'original' }]) + } finally { + db.close() + dropDatabaseIfExists(dbName) + } + }) + + it('serializes direct native async result metadata', async () => { + const dbName = 'native-concurrent-inserts' + dropDatabaseIfExists(dbName) + NitroSQLite.native.open(dbName) + + try { + NitroSQLite.native.execute( + dbName, + 'CREATE TABLE Item (id INTEGER PRIMARY KEY AUTOINCREMENT, value TEXT)', + ) + const results = await Promise.all( + Array.from({ length: 24 }, (_, index) => + NitroSQLite.native.executeAsync( + dbName, + 'INSERT INTO Item (value) VALUES (?)', + [`value-${index}`], + ), + ), + ) + + const insertIds = results + .map((result) => result.insertId) + .sort((a, b) => (a ?? 0) - (b ?? 0)) + expect(insertIds).toEqual( + Array.from({ length: 24 }, (_, index) => index + 1), + ) + + const storedRows = NitroSQLite.native.execute( + dbName, + 'SELECT id, value FROM Item', + ).results + results.forEach((result, index) => { + const storedRow = storedRows.find((row) => row.id === result.insertId) + expect(storedRow?.value).toBe(`value-${index}`) + }) + } finally { + NitroSQLite.native.close(dbName) + dropDatabaseIfExists(dbName) + } + }) + + it('does not reroute native async work after close and reopen', async () => { + const dbName = 'native-close-reopen' + dropDatabaseIfExists(dbName) + dropDatabaseIfExists(dbName, '..') + NitroSQLite.native.open(dbName) + + try { + NitroSQLite.native.execute( + dbName, + 'CREATE TABLE ConnectionMarker (value TEXT NOT NULL)', + ) + NitroSQLite.native.execute( + dbName, + 'INSERT INTO ConnectionMarker (value) VALUES (?)', + ['original'], + ) + + const pending = NitroSQLite.native.executeAsync( + dbName, + 'WITH RECURSIVE counter(value) AS (VALUES(0) UNION ALL SELECT value + 1 FROM counter WHERE value < 100000) SELECT ConnectionMarker.value, sum(counter.value) AS total FROM ConnectionMarker, counter', + ) + + NitroSQLite.native.close(dbName) + NitroSQLite.native.open(dbName, '..') + NitroSQLite.native.execute( + dbName, + 'CREATE TABLE ConnectionMarker (value TEXT NOT NULL)', + ) + NitroSQLite.native.execute( + dbName, + 'INSERT INTO ConnectionMarker (value) VALUES (?)', + ['replacement'], + ) + + try { + const result = await pending + expect(result.results[0]?.value).toBe('original') + } catch (error) { + expect((error as Error).message).toContain('not open') + } + + expect( + NitroSQLite.execute<{ value: string }>( + dbName, + 'SELECT value FROM ConnectionMarker', + ).results, + ).toEqual([{ value: 'replacement' }]) + } finally { + dropDatabaseIfExists(dbName) + dropDatabaseIfExists(dbName, '..') + } + }) + + it('rejects missing native async databases asynchronously', async () => { + const dbName = 'native-async-missing-database' + dropDatabaseIfExists(dbName) + + const pending = NitroSQLite.native.executeAsync(dbName, 'SELECT 1') + let asyncError: unknown + try { + await pending + } catch (error) { + asyncError = error + } + + expect(asyncError).toBeInstanceOf(Error) + expect((asyncError as Error).message).toContain('not open') + }) }) } diff --git a/example/tests/unit/specs/operations/transaction.spec.ts b/example/tests/unit/specs/operations/transaction.spec.ts index bc0e3ad3..f835f786 100644 --- a/example/tests/unit/specs/operations/transaction.spec.ts +++ b/example/tests/unit/specs/operations/transaction.spec.ts @@ -196,9 +196,10 @@ export default function registerTransactionUnitTests() { [id, name, age, networth], ) tx.rollback() - const res = testDb.execute('SELECT * FROM User') - expect(res.rows?._array).toEqual([]) }) + + const res = testDb.execute('SELECT * FROM User') + expect(res.rows?._array).toEqual([]) }) it('Transaction, rejects on callback error', async () => { diff --git a/example/tests/unit/specs/typeorm.spec.ts b/example/tests/unit/specs/typeorm.spec.ts index f4879b91..13e51b4d 100644 --- a/example/tests/unit/specs/typeorm.spec.ts +++ b/example/tests/unit/specs/typeorm.spec.ts @@ -68,5 +68,31 @@ export default function registerTypeORMUnitTests() { expect(books).toHaveLength(1) expect(books[0]?.title).toBe('Test Book') }) + + it('runs parallel reads through the TypeORM driver', async () => { + const users = Array.from({ length: 20 }, (_, index) => + userRepository.create({ + name: `Concurrent User ${index}`, + age: index, + networth: index * 100, + metadata: { nickname: `concurrent-${index}` }, + avatar: new Uint8Array([index]).buffer, + }), + ) + await userRepository.save(users) + + const reads = await Promise.all( + Array.from({ length: 32 }, () => + userRepository.find({ order: { age: 'ASC' } }), + ), + ) + + for (const result of reads) { + expect(result).toHaveLength(users.length) + expect(result.map((user) => user.age)).toEqual( + users.map((user) => user.age), + ) + } + }) }) } diff --git a/packages/react-native-nitro-sqlite/cpp/NitroSQLiteException.hpp b/packages/react-native-nitro-sqlite/cpp/NitroSQLiteException.hpp index e63f5d70..3c13b1fd 100644 --- a/packages/react-native-nitro-sqlite/cpp/NitroSQLiteException.hpp +++ b/packages/react-native-nitro-sqlite/cpp/NitroSQLiteException.hpp @@ -46,6 +46,11 @@ class NitroSQLiteException : public std::exception { return this->_exceptionString.c_str(); } + static NitroSQLiteException DatabaseAlreadyOpen(const std::string& dbName) { + return NitroSQLiteException(NitroSQLiteExceptionType::DatabaseCannotBeOpened, + "Database " + dbName + " is already open. There is already a connection to the database."); + } + static NitroSQLiteException DatabaseNotOpen(const std::string& dbName) { return NitroSQLiteException(NitroSQLiteExceptionType::UnableToAttachToDatabase, dbName + " is not open"); } diff --git a/packages/react-native-nitro-sqlite/cpp/hybridObjects/HybridNitroSQLite.cpp b/packages/react-native-nitro-sqlite/cpp/hybridObjects/HybridNitroSQLite.cpp index 60a1d63f..74983e0a 100644 --- a/packages/react-native-nitro-sqlite/cpp/hybridObjects/HybridNitroSQLite.cpp +++ b/packages/react-native-nitro-sqlite/cpp/hybridObjects/HybridNitroSQLite.cpp @@ -6,6 +6,7 @@ #include "macros.hpp" #include "operations.hpp" #include "sqliteExecuteBatch.hpp" +#include #include #include #include @@ -101,10 +102,16 @@ std::shared_ptr HybridNitroSQLite::execute(con std::shared_ptr>> HybridNitroSQLite::executeAsync(const std::string& dbName, const std::string& query, const std::optional& params) { const auto copiedParams = copyArrayBufferParamsForBackground(params); + SQLiteConnectionPtr connection; + try { + connection = sqliteGetOpenDatabase(dbName); + } catch (...) { + return Promise>::rejected(std::current_exception()); + } return Promise>::async( - [=, this]() -> std::shared_ptr { - auto result = sqliteExecute(dbName, query, copiedParams); + [connection, query, copiedParams]() -> std::shared_ptr { + auto result = sqliteExecute(connection, query, copiedParams); return result; }); }; @@ -122,9 +129,15 @@ std::shared_ptr> HybridNitroSQLite::executeBatchAsync( // ArrayBuffers into native buffers before going off-thread. const auto commands = batchParamsToCommands(batchParams); const auto copiedCommands = copyArrayBufferParamsForBackground(commands); + SQLiteConnectionPtr connection; + try { + connection = sqliteGetOpenDatabase(dbName); + } catch (...) { + return Promise::rejected(std::current_exception()); + } - return Promise::async([=, this]() -> BatchQueryResult { - auto result = sqliteExecuteBatch(dbName, copiedCommands); + return Promise::async([connection, copiedCommands]() -> BatchQueryResult { + auto result = sqliteExecuteBatch(connection, copiedCommands); return BatchQueryResult(result.rowsAffected); }); }; @@ -135,9 +148,15 @@ FileLoadResult HybridNitroSQLite::loadFile(const std::string& dbName, const std: }; std::shared_ptr> HybridNitroSQLite::loadFileAsync(const std::string& dbName, const std::string& location) { - return Promise::async([=, this]() -> FileLoadResult { - auto result = loadFile(dbName, location); - return result; + SQLiteConnectionPtr connection; + try { + connection = sqliteGetOpenDatabase(dbName); + } catch (...) { + return Promise::rejected(std::current_exception()); + } + return Promise::async([connection, location]() -> FileLoadResult { + const auto result = importSqlFile(connection, location); + return FileLoadResult(result.commands, result.rowsAffected); }); }; diff --git a/packages/react-native-nitro-sqlite/cpp/importSqlFile.cpp b/packages/react-native-nitro-sqlite/cpp/importSqlFile.cpp index 16857ad4..e38f06f0 100644 --- a/packages/react-native-nitro-sqlite/cpp/importSqlFile.cpp +++ b/packages/react-native-nitro-sqlite/cpp/importSqlFile.cpp @@ -11,21 +11,26 @@ namespace margelo::rnnitrosqlite { SQLiteOperationResult importSqlFile(const std::string& dbName, const std::string& fileLocation) { + return importSqlFile(sqliteGetOpenDatabase(dbName), fileLocation); +} + +SQLiteOperationResult importSqlFile(const SQLiteConnectionPtr& connection, const std::string& fileLocation) { + std::lock_guard lock(connection->mutex); std::string line; std::ifstream sqFile(fileLocation); if (sqFile.is_open()) { try { int rowsAffected = 0; int commands = 0; - sqliteExecuteCommand(dbName, "BEGIN EXCLUSIVE TRANSACTION"); + sqliteExecuteCommand(connection, "BEGIN EXCLUSIVE TRANSACTION"); while (std::getline(sqFile, line, '\n')) { if (!line.empty()) { try { - SQLiteOperationResult result = sqliteExecuteCommand(dbName, line); + SQLiteOperationResult result = sqliteExecuteCommand(connection, line); rowsAffected += result.rowsAffected; commands++; } catch (NitroSQLiteException& e) { - sqliteExecuteCommand(dbName, "ROLLBACK"); + sqliteExecuteCommand(connection, "ROLLBACK"); sqFile.close(); throw NitroSQLiteException::CouldNotLoadFile(fileLocation, "Transaction was rolled back"); } @@ -33,11 +38,11 @@ SQLiteOperationResult importSqlFile(const std::string& dbName, const std::string } sqFile.close(); - sqliteExecuteCommand(dbName, "COMMIT"); + sqliteExecuteCommand(connection, "COMMIT"); return {.rowsAffected = rowsAffected, .commands = commands}; } catch (...) { sqFile.close(); - sqliteExecuteCommand(dbName, "ROLLBACK"); + sqliteExecuteCommand(connection, "ROLLBACK"); throw NitroSQLiteException(NitroSQLiteExceptionType::UnknownError, "Unexpected error. Transaction was rolled back"); } } else { diff --git a/packages/react-native-nitro-sqlite/cpp/importSqlFile.hpp b/packages/react-native-nitro-sqlite/cpp/importSqlFile.hpp index 0416a10c..766c6779 100644 --- a/packages/react-native-nitro-sqlite/cpp/importSqlFile.hpp +++ b/packages/react-native-nitro-sqlite/cpp/importSqlFile.hpp @@ -7,9 +7,13 @@ #pragma once #include "types.hpp" +#include namespace margelo::rnnitrosqlite { +struct SQLiteConnection; + SQLiteOperationResult importSqlFile(const std::string& dbName, const std::string& fileLocation); +SQLiteOperationResult importSqlFile(const std::shared_ptr& connection, const std::string& fileLocation); -} +} // namespace margelo::rnnitrosqlite diff --git a/packages/react-native-nitro-sqlite/cpp/operations.cpp b/packages/react-native-nitro-sqlite/cpp/operations.cpp index ae61dc65..8c99db01 100644 --- a/packages/react-native-nitro-sqlite/cpp/operations.cpp +++ b/packages/react-native-nitro-sqlite/cpp/operations.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -29,9 +30,39 @@ namespace margelo::rnnitrosqlite { static constexpr double kInt64MinAsDouble = static_cast(std::numeric_limits::min()); static constexpr double kInt64UpperBoundAsDouble = -kInt64MinAsDouble; -std::map dbMap = std::map(); +namespace { + + std::map dbMap; + std::mutex dbMapMutex; + std::mutex dbLifecycleMutex; + +} // namespace + +SQLiteConnection::SQLiteConnection(std::string connectionName, sqlite3* database) : name(std::move(connectionName)), database(database) {} + +SQLiteConnection::~SQLiteConnection() { + close(); +} + +void SQLiteConnection::close() noexcept { + std::lock_guard lock(mutex); + if (database == nullptr) { + return; + } + + sqlite3_close_v2(database); + database = nullptr; +} void sqliteOpenDb(const std::string& dbName, const std::string& docPath) { + std::lock_guard lifecycleLock(dbLifecycleMutex); + { + std::lock_guard lock(dbMapMutex); + if (dbMap.contains(dbName)) { + throw NitroSQLiteException::DatabaseAlreadyOpen(dbName); + } + } + #ifdef NITRO_SQLITE_VEC // Register before opening so the connection exposes vec0 + vec_*. margelo::rnnitrosqlitevec::registerVectorExtensions(); @@ -41,37 +72,54 @@ void sqliteOpenDb(const std::string& dbName, const std::string& docPath) { int sqlOpenFlags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX; - sqlite3* db; - int exit = 0; - exit = sqlite3_open_v2(dbPath.c_str(), &db, sqlOpenFlags, nullptr); + sqlite3* rawDatabase = nullptr; + const int openStatus = sqlite3_open_v2(dbPath.c_str(), &rawDatabase, sqlOpenFlags, nullptr); + std::unique_ptr database(rawDatabase, sqlite3_close_v2); + + if (openStatus != SQLITE_OK) { + const std::string errorMessage = rawDatabase == nullptr ? sqlite3_errstr(openStatus) : sqlite3_errmsg(rawDatabase); + throw NitroSQLiteException(NitroSQLiteExceptionType::DatabaseCannotBeOpened, errorMessage); + } - if (exit != SQLITE_OK) { - throw NitroSQLiteException(NitroSQLiteExceptionType::DatabaseCannotBeOpened, sqlite3_errmsg(db)); - } else { - dbMap[dbName] = db; + auto connection = std::make_shared(dbName, database.get()); + database.release(); + { + std::lock_guard lock(dbMapMutex); + const bool inserted = dbMap.emplace(dbName, connection).second; + if (!inserted) { + throw NitroSQLiteException::DatabaseAlreadyOpen(dbName); + } } } void sqliteCloseDb(const std::string& dbName) { + std::lock_guard lifecycleLock(dbLifecycleMutex); + SQLiteConnectionPtr connection; + { + std::lock_guard lock(dbMapMutex); + auto iterator = dbMap.find(dbName); + if (iterator == dbMap.end()) { + throw NitroSQLiteException::DatabaseNotOpen(dbName); + } - if (dbMap.count(dbName) == 0) { - throw NitroSQLiteException::DatabaseNotOpen(dbName); + connection = std::move(iterator->second); + dbMap.erase(iterator); } - sqlite3* db = dbMap[dbName]; - - sqlite3_close_v2(db); - - dbMap.erase(dbName); + connection->close(); } void sqliteCloseAll() { - for (auto const& x : dbMap) { - // In certain cases, this will return SQLITE_OK, mark the database connection as an unusable "zombie", - // and deallocate the connection later. - sqlite3_close_v2(x.second); + std::lock_guard lifecycleLock(dbLifecycleMutex); + std::map connections; + { + std::lock_guard lock(dbMapMutex); + connections.swap(dbMap); + } + + for (const auto& [_, connection] : connections) { + connection->close(); } - dbMap.clear(); } void sqliteAttachDb(const std::string& mainDBName, const std::string& docPath, const std::string& databaseToAttach, @@ -105,15 +153,26 @@ void sqliteDetachDb(const std::string& mainDBName, const std::string& alias) { } void sqliteRemoveDb(const std::string& dbName, const std::string& docPath) { - if (dbMap.count(dbName) == 1) { - sqliteCloseDb(dbName); - } - - std::string dbFilePath = get_db_path(dbName, docPath); + std::lock_guard lifecycleLock(dbLifecycleMutex); + const std::string dbFilePath = get_db_path(dbName, docPath); if (!file_exists(dbFilePath)) { throw NitroSQLiteException::DatabaseFileNotFound(dbFilePath); } + SQLiteConnectionPtr connection; + { + std::lock_guard lock(dbMapMutex); + auto iterator = dbMap.find(dbName); + if (iterator != dbMap.end()) { + connection = std::move(iterator->second); + dbMap.erase(iterator); + } + } + + if (connection) { + connection->close(); + } + remove(dbFilePath.c_str()); } @@ -155,14 +214,6 @@ namespace { using SQLiteStatement = std::unique_ptr; - sqlite3* getOpenDatabase(const std::string& dbName) { - if (dbMap.count(dbName) == 0) { - throw NitroSQLiteException::DatabaseNotOpen(dbName); - } - - return dbMap[dbName]; - } - SQLiteStatement prepareStatement(sqlite3* db, const std::string& query, const std::optional& params) { sqlite3_stmt* rawStatement = nullptr; int statementStatus = sqlite3_prepare_v2(db, query.c_str(), -1, &rawStatement, nullptr); @@ -199,9 +250,29 @@ namespace { } // namespace +SQLiteConnectionPtr sqliteGetOpenDatabase(const std::string& dbName) { + std::lock_guard lock(dbMapMutex); + auto iterator = dbMap.find(dbName); + if (iterator == dbMap.end()) { + throw NitroSQLiteException::DatabaseNotOpen(dbName); + } + + return iterator->second; +} + std::shared_ptr sqliteExecute(const std::string& dbName, const std::string& query, const std::optional& params) { - auto db = getOpenDatabase(dbName); + return sqliteExecute(sqliteGetOpenDatabase(dbName), query, params); +} + +std::shared_ptr sqliteExecute(const SQLiteConnectionPtr& connection, const std::string& query, + const std::optional& params) { + std::lock_guard lock(connection->mutex); + sqlite3* db = connection->database; + if (db == nullptr) { + throw NitroSQLiteException::DatabaseNotOpen(connection->name); + } + auto statement = prepareStatement(db, query, params); SQLiteQueryResults results; @@ -265,7 +336,17 @@ std::shared_ptr sqliteExecute(const std::string& d SQLiteOperationResult sqliteExecuteCommand(const std::string& dbName, const std::string& query, const std::optional& params) { - auto db = getOpenDatabase(dbName); + return sqliteExecuteCommand(sqliteGetOpenDatabase(dbName), query, params); +} + +SQLiteOperationResult sqliteExecuteCommand(const SQLiteConnectionPtr& connection, const std::string& query, + const std::optional& params) { + std::lock_guard lock(connection->mutex); + sqlite3* db = connection->database; + if (db == nullptr) { + throw NitroSQLiteException::DatabaseNotOpen(connection->name); + } + auto statement = prepareStatement(db, query, params); bool isReadOnly = sqlite3_stmt_readonly(statement.get()) != 0; diff --git a/packages/react-native-nitro-sqlite/cpp/operations.hpp b/packages/react-native-nitro-sqlite/cpp/operations.hpp index 49fce8ca..549d68e5 100644 --- a/packages/react-native-nitro-sqlite/cpp/operations.hpp +++ b/packages/react-native-nitro-sqlite/cpp/operations.hpp @@ -2,9 +2,32 @@ #include "hybridObjects/HybridNitroSQLiteQueryResult.hpp" #include "types.hpp" +#include +#include +#include +#include namespace margelo::rnnitrosqlite { +// Calls against one connection are serialized by `mutex`. Separate connections +// intentionally remain independent, so SQLITE_THREADSAFE=0 still requires the +// caller to serialize SQLite calls globally. +struct SQLiteConnection final { + SQLiteConnection(std::string name, sqlite3* database); + ~SQLiteConnection(); + + SQLiteConnection(const SQLiteConnection&) = delete; + SQLiteConnection& operator=(const SQLiteConnection&) = delete; + + void close() noexcept; + + const std::string name; + sqlite3* database; + std::recursive_mutex mutex; +}; + +using SQLiteConnectionPtr = std::shared_ptr; + void sqliteOpenDb(const std::string& dbName, const std::string& docPath); void sqliteCloseDb(const std::string& dbName); @@ -16,11 +39,17 @@ void sqliteAttachDb(const std::string& mainDBName, const std::string& docPath, c void sqliteDetachDb(const std::string& mainDBName, const std::string& alias); +SQLiteConnectionPtr sqliteGetOpenDatabase(const std::string& dbName); + std::shared_ptr sqliteExecute(const std::string& dbName, const std::string& query, const std::optional& params); +std::shared_ptr sqliteExecute(const SQLiteConnectionPtr& connection, const std::string& query, + const std::optional& params); SQLiteOperationResult sqliteExecuteCommand(const std::string& dbName, const std::string& query, const std::optional& params = std::nullopt); +SQLiteOperationResult sqliteExecuteCommand(const SQLiteConnectionPtr& connection, const std::string& query, + const std::optional& params = std::nullopt); void sqliteCloseAll(); diff --git a/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.cpp b/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.cpp index a3731250..0e3d92ca 100644 --- a/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.cpp +++ b/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.cpp @@ -33,6 +33,11 @@ std::vector batchParamsToCommands(const std::vector& commands) { + return sqliteExecuteBatch(sqliteGetOpenDatabase(dbName), commands); +} + +SQLiteOperationResult sqliteExecuteBatch(const SQLiteConnectionPtr& connection, const std::vector& commands) { + std::lock_guard lock(connection->mutex); size_t commandCount = commands.size(); if (commandCount <= 0) { throw NitroSQLiteException(NitroSQLiteExceptionType::NoBatchCommandsProvided, "No SQL batch commands provided"); @@ -40,13 +45,13 @@ SQLiteOperationResult sqliteExecuteBatch(const std::string& dbName, const std::v try { int rowsAffected = 0; - sqliteExecuteCommand(dbName, "BEGIN EXCLUSIVE TRANSACTION"); + sqliteExecuteCommand(connection, "BEGIN EXCLUSIVE TRANSACTION"); for (const auto& command : commands) { - auto result = sqliteExecuteCommand(dbName, command.sql, command.params); + auto result = sqliteExecuteCommand(connection, command.sql, command.params); rowsAffected += result.rowsAffected; } - sqliteExecuteCommand(dbName, "COMMIT"); + sqliteExecuteCommand(connection, "COMMIT"); return { .rowsAffected = rowsAffected, .commands = (int)commandCount, @@ -54,7 +59,7 @@ SQLiteOperationResult sqliteExecuteBatch(const std::string& dbName, const std::v } catch (NitroSQLiteException& e) { // Roll back exactly once; a failed ROLLBACK must not mask the original error. try { - sqliteExecuteCommand(dbName, "ROLLBACK"); + sqliteExecuteCommand(connection, "ROLLBACK"); } catch (...) { // ignore — surface the original error below } diff --git a/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.hpp b/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.hpp index 4b07b89d..aa68152c 100644 --- a/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.hpp +++ b/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.hpp @@ -5,12 +5,15 @@ #include "BatchQueryCommand.hpp" #include "types.hpp" +#include using namespace facebook; using namespace margelo::nitro; namespace margelo::rnnitrosqlite { +struct SQLiteConnection; + struct BatchQuery { std::string sql; std::optional params; @@ -26,5 +29,6 @@ std::vector batchParamsToCommands(const std::vector& commands); +SQLiteOperationResult sqliteExecuteBatch(const std::shared_ptr& connection, const std::vector& commands); } // namespace margelo::rnnitrosqlite diff --git a/packages/react-native-nitro-sqlite/src/DatabaseQueue.ts b/packages/react-native-nitro-sqlite/src/DatabaseQueue.ts index bd485a61..1ad4d258 100644 --- a/packages/react-native-nitro-sqlite/src/DatabaseQueue.ts +++ b/packages/react-native-nitro-sqlite/src/DatabaseQueue.ts @@ -28,8 +28,8 @@ export function closeDatabaseQueue(dbName: string) { const databaseQueue = getDatabaseQueue(dbName) if (databaseQueue.inProgress || databaseQueue.queue.length > 0) { - console.warn( - `Database queue for ${dbName} has operations in the queue. Closing anyway.`, + throw new NitroSQLiteError( + `Cannot close database ${dbName}. The database is busy with another operation.`, ) } @@ -54,14 +54,6 @@ export function getDatabaseQueue(dbName: string) { return queue } -export function openDatabase(dbName: string) { - databaseQueues.set(dbName, { queue: [], inProgress: false }) -} - -export function closeDatabase(dbName: string) { - databaseQueues.delete(dbName) -} - export function queueOperationAsync( dbName: string, callback: () => Promise, @@ -77,7 +69,7 @@ export function queueOperationAsync( reject(error) } finally { databaseQueue.inProgress = false - startOperationAsync(dbName) + startOperationAsync(databaseQueue) } } @@ -86,13 +78,11 @@ export function queueOperationAsync( } databaseQueue.queue.push(operation) - startOperationAsync(dbName) + startOperationAsync(databaseQueue) }) } -function startOperationAsync(dbName: string) { - const queue = getDatabaseQueue(dbName) - +function startOperationAsync(queue: DatabaseQueue) { // Queue is empty or in progress. Bail out. if (queue.inProgress || queue.queue.length === 0) { return @@ -106,10 +96,10 @@ function startOperationAsync(dbName: string) { }) } -export function startOperationSync< - OperationCallback extends () => Result, - Result = void, ->(dbName: string, callback: OperationCallback) { +export function startOperationSync( + dbName: string, + callback: () => Result, +): Result { const databaseQueue = getDatabaseQueue(dbName) // Database is busy - cannot execute synchronously diff --git a/packages/react-native-nitro-sqlite/src/operations/execute.ts b/packages/react-native-nitro-sqlite/src/operations/execute.ts index a12b8061..bc75a84a 100644 --- a/packages/react-native-nitro-sqlite/src/operations/execute.ts +++ b/packages/react-native-nitro-sqlite/src/operations/execute.ts @@ -2,11 +2,36 @@ import { HybridNitroSQLite } from '../nitro' import type { QueryResult, QueryResultRow, SQLiteQueryParams } from '../types' import NitroSQLiteError from '../NitroSQLiteError' import type { NitroSQLiteQueryResult } from '../specs/NitroSQLiteQueryResult.nitro' +import { + isDatabaseOpen, + queueOperationAsync, + startOperationSync, +} from '../DatabaseQueue' export function execute( dbName: string, query: string, params?: SQLiteQueryParams, +): QueryResult { + if (!isDatabaseOpen(dbName)) { + return executeNative(dbName, query, params) + } + + return executeManaged(dbName, query, params) +} + +export function executeManaged( + dbName: string, + query: string, + params?: SQLiteQueryParams, +): QueryResult { + return startOperationSync(dbName, () => executeNative(dbName, query, params)) +} + +export function executeNative( + dbName: string, + query: string, + params?: SQLiteQueryParams, ): QueryResult { try { const nativeResult = HybridNitroSQLite.execute(dbName, query, params) @@ -20,6 +45,28 @@ export async function executeAsync( dbName: string, query: string, params?: SQLiteQueryParams, +): Promise> { + if (!isDatabaseOpen(dbName)) { + return executeAsyncNative(dbName, query, params) + } + + return executeAsyncManaged(dbName, query, params) +} + +export async function executeAsyncManaged( + dbName: string, + query: string, + params?: SQLiteQueryParams, +): Promise> { + return queueOperationAsync(dbName, () => + executeAsyncNative(dbName, query, params), + ) +} + +export async function executeAsyncNative( + dbName: string, + query: string, + params?: SQLiteQueryParams, ): Promise> { try { const nativeResult = await HybridNitroSQLite.executeAsync( diff --git a/packages/react-native-nitro-sqlite/src/operations/session.ts b/packages/react-native-nitro-sqlite/src/operations/session.ts index 391407f2..6b4d5a6d 100644 --- a/packages/react-native-nitro-sqlite/src/operations/session.ts +++ b/packages/react-native-nitro-sqlite/src/operations/session.ts @@ -9,51 +9,97 @@ import type { QueryResultRow, QueryResult, } from '../types' -import { execute, executeAsync } from './execute' +import { executeAsyncManaged, executeManaged } from './execute' import { executeBatch, executeBatchAsync } from './executeBatch' import NitroSQLiteError from '../NitroSQLiteError' -import { closeDatabaseQueue, openDatabaseQueue } from '../DatabaseQueue' +import { + closeDatabaseQueue, + isDatabaseOpen, + openDatabaseQueue, + queueOperationAsync, + startOperationSync, +} from '../DatabaseQueue' export function open( options: NitroSQLiteConnectionOptions, ): NitroSQLiteConnection { + openDatabaseQueue(options.name) + try { HybridNitroSQLite.open(options.name, options.location) - openDatabaseQueue(options.name) } catch (error) { + closeDatabaseQueue(options.name) throw NitroSQLiteError.fromError(error) } return { close: () => { try { - HybridNitroSQLite.close(options.name) + startOperationSync(options.name, () => + HybridNitroSQLite.close(options.name), + ) + closeDatabaseQueue(options.name) + } catch (error) { + throw NitroSQLiteError.fromError(error) + } + }, + delete: () => { + try { + if (!isDatabaseOpen(options.name)) { + HybridNitroSQLite.drop(options.name, options.location) + return + } + + startOperationSync(options.name, () => + HybridNitroSQLite.drop(options.name, options.location), + ) closeDatabaseQueue(options.name) } catch (error) { throw NitroSQLiteError.fromError(error) } }, - delete: () => HybridNitroSQLite.drop(options.name, options.location), attach: (dbNameToAttach: string, alias: string, location?: string) => - HybridNitroSQLite.attach(options.name, dbNameToAttach, alias, location), - detach: (alias: string) => HybridNitroSQLite.detach(options.name, alias), + runSyncOperation(options.name, () => + HybridNitroSQLite.attach(options.name, dbNameToAttach, alias, location), + ), + detach: (alias: string) => + runSyncOperation(options.name, () => + HybridNitroSQLite.detach(options.name, alias), + ), transaction: (fn: (tx: Transaction) => Promise) => transaction(options.name, fn), execute: ( query: string, params?: SQLiteQueryParams, - ): QueryResult => execute(options.name, query, params), + ): QueryResult => executeManaged(options.name, query, params), executeAsync: ( query: string, params?: SQLiteQueryParams, - ): Promise> => executeAsync(options.name, query, params), + ): Promise> => + executeAsyncManaged(options.name, query, params), executeBatch: (commands: BatchQueryCommand[]) => executeBatch(options.name, commands), executeBatchAsync: (commands: BatchQueryCommand[]) => executeBatchAsync(options.name, commands), loadFile: (location: string) => - HybridNitroSQLite.loadFile(options.name, location), + runSyncOperation(options.name, () => + HybridNitroSQLite.loadFile(options.name, location), + ), loadFileAsync: (location: string) => - HybridNitroSQLite.loadFileAsync(options.name, location), + queueOperationAsync(options.name, async () => { + try { + return await HybridNitroSQLite.loadFileAsync(options.name, location) + } catch (error) { + throw NitroSQLiteError.fromError(error) + } + }), + } +} + +function runSyncOperation(dbName: string, callback: () => Result) { + try { + return startOperationSync(dbName, callback) + } catch (error) { + throw NitroSQLiteError.fromError(error) } } diff --git a/packages/react-native-nitro-sqlite/src/operations/transaction.ts b/packages/react-native-nitro-sqlite/src/operations/transaction.ts index ae969b3c..25f68b2d 100644 --- a/packages/react-native-nitro-sqlite/src/operations/transaction.ts +++ b/packages/react-native-nitro-sqlite/src/operations/transaction.ts @@ -5,7 +5,7 @@ import type { QueryResult, QueryResultRow, } from '../types' -import { execute, executeAsync } from './execute' +import { executeAsyncNative, executeNative } from './execute' import NitroSQLiteError from '../NitroSQLiteError' export const transaction = async ( @@ -26,7 +26,7 @@ export const transaction = async ( `Cannot execute query on finalized transaction: ${dbName}`, ) } - return execute(dbName, query, params) + return executeNative(dbName, query, params) } const executeAsyncOnTransaction = ( @@ -38,7 +38,7 @@ export const transaction = async ( `Cannot execute query on finalized transaction: ${dbName}`, ) } - return executeAsync(dbName, query, params) + return executeAsyncNative(dbName, query, params) } const commit = () => { @@ -48,7 +48,7 @@ export const transaction = async ( ) } isFinished = true - return execute(dbName, 'COMMIT') + return executeNative(dbName, 'COMMIT') } const rollback = () => { @@ -58,12 +58,12 @@ export const transaction = async ( ) } isFinished = true - return execute(dbName, 'ROLLBACK') + return executeNative(dbName, 'ROLLBACK') } return await queueOperationAsync(dbName, async () => { try { - await executeAsync( + await executeAsyncNative( dbName, isExclusive ? 'BEGIN EXCLUSIVE TRANSACTION' : 'BEGIN TRANSACTION', )