Open DB connections with SQLITE_OPEN_FULLMUTEX. Ensure connections are free when attaching/detaching databases. Fix test for throwing exception.

Author: stevensJourney

cpp/ConnectionPool.cpp

@@ -22,13 +22,13 @@ ConnectionPool::ConnectionPool(std::string dbName, std::string docPath,
   // Open the write connection
   genericSqliteOpenDb(dbName, docPath, &writeConnection.connection,
                       SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE |
-                          SQLITE_OPEN_NOMUTEX);
+                          SQLITE_OPEN_FULLMUTEX);
 
   // Open the read connections
   for (int i = 0; i < maxReads; i++) {
     sqlite3 *db;
     genericSqliteOpenDb(dbName, docPath, &db,
-                        SQLITE_OPEN_READONLY | SQLITE_OPEN_NOMUTEX);
+                        SQLITE_OPEN_READONLY | SQLITE_OPEN_FULLMUTEX);
     struct ConnectionState readCon;
     readCon.connection = db;
     readCon.currentLockId = EMPTY_LOCK_ID;
@@ -206,9 +206,19 @@ SQLiteOPResult ConnectionPool::attachDatabase(std::string const dbFileName,
 
   auto dbConnections = getAllConnections();
 
-  for (auto &connection : dbConnections) {
+  for (auto &connectionState : dbConnections) {
+    if (connectionState.currentLockId != EMPTY_LOCK_ID) {
+      return SQLiteOPResult{
+          .type = SQLiteError,
+          .errorMessage = dbName + " was unable to attach another database: " +
+                          "Some DB connections were locked",
+      };
+    }
+  }
+
+  for (auto &connectionState : dbConnections) {
     SequelLiteralUpdateResult result =
-        sqliteExecuteLiteralWithDB(connection, statement);
+        sqliteExecuteLiteralWithDB(connectionState.connection, statement);
     if (result.type == SQLiteError) {
       // Revert change on any successful connections
       detachDatabase(alias);
@@ -233,9 +243,19 @@ SQLiteOPResult ConnectionPool::detachDatabase(std::string const alias) {
   string statement = "DETACH DATABASE " + alias;
   auto dbConnections = getAllConnections();
 
-  for (auto &connection : dbConnections) {
+  for (auto &connectionState : dbConnections) {
+    if (connectionState.currentLockId != EMPTY_LOCK_ID) {
+      return SQLiteOPResult{
+          .type = SQLiteError,
+          .errorMessage = dbName + " was unable to detach another database: " +
+                          "Some DB connections were locked",
+      };
+    }
+  }
+
+  for (auto &connectionState : dbConnections) {
     SequelLiteralUpdateResult result =
-        sqliteExecuteLiteralWithDB(connection, statement);
+        sqliteExecuteLiteralWithDB(connectionState.connection, statement);
     if (result.type == SQLiteError) {
       return SQLiteOPResult{
           .type = SQLiteError,
@@ -294,11 +314,11 @@ ConnectionPool::importSQLFile(std::string fileLocation) {
 
 // ===================== Private ===============
 
-std::vector<sqlite3 *> ConnectionPool::getAllConnections() {
-  std::vector<sqlite3 *> result;
-  result.push_back(writeConnection.connection);
+std::vector<ConnectionState> ConnectionPool::getAllConnections() {
+  std::vector<ConnectionState> result;
+  result.push_back(writeConnection);
   for (int i = 0; i < maxReads; i++) {
-    result.push_back(readConnections[i].connection);
+    result.push_back(readConnections[i]);
   }
   return result;
 }

cpp/ConnectionPool.h

@@ -139,7 +139,7 @@ class ConnectionPool {
   SequelBatchOperationResult importSQLFile(std::string fileLocation);
 
 private:
-  std::vector<sqlite3 *> getAllConnections();
+  std::vector<ConnectionState> getAllConnections();
 
   void activateContext(ConnectionState *state, ConnectionLockId contextId);
 

src/setup-open.ts

@@ -12,6 +12,7 @@ import {
 } from './types';
 
 import uuid from 'uuid';
+import _ from 'lodash';
 import { enhanceQueryResult } from './utils';
 import { registerUpdateHook } from './table-updates';
 
@@ -111,9 +112,11 @@ export function setupOpen(QuickSQLite: ISQLite) {
             callback: async (context: LockContext) => {
               try {
                 const res = await callback(context);
-                resolve(res);
+
+                // Ensure that we only resolve after locks are freed
+                _.defer(() => resolve(res));
               } catch (ex) {
-                reject(ex);
+                _.defer(() => reject(ex));
               }
             }
           } as LockCallbackRecord);

src/types.ts

@@ -142,7 +142,13 @@ export type QuickSQLiteConnection = {
   writeLock: <T>(callback: (context: LockContext) => Promise<T>, options?: LockOptions) => Promise<T>;
   writeTransaction: <T>(callback: (context: TransactionContext) => Promise<T>, options?: LockOptions) => Promise<T>;
   delete: () => void;
+  /**
+   * The attach method should only be called if there are no active locks/transactions.
+   */
   attach: (dbNameToAttach: string, alias: string, location?: string) => void;
+  /**
+   * The detach method should only be called if there are no active locks/transactions.
+   */
   detach: (alias: string) => void;
   executeBatchAsync: (commands: SQLBatchTuple[]) => Promise<BatchQueryResult>;
   loadFileAsync: (location: string) => Promise<FileLoadResult>;

tests/tests/sqlite/rawQueries.spec.ts

@@ -1,5 +1,5 @@
 import Chance from 'chance';
-import { open, QuickSQLiteConnection, SQLBatchTuple, UpdateNotification } from 'react-native-quick-sqlite';
+import { open, QuickSQLite, QuickSQLiteConnection, SQLBatchTuple, UpdateNotification } from 'react-native-quick-sqlite';
 import { beforeEach, describe, it } from '../mocha/MochaRNAdapter';
 import chai from 'chai';
 
@@ -86,15 +86,17 @@ export function registerBaseTests() {
     });
 
     it('Failed insert', async () => {
-      const { id, name, age, networth } = generateUserInfo();
-      // expect(
+      const id = chance.string(); // Setting the id to a string will throw an exception, it expects an int
+      const { name, age, networth } = generateUserInfo();
+      let errorThrown = false;
       try {
         await db.execute('INSERT INTO User (id, name, age, networth) VALUES(?, ?, ?, ?)', [id, name, age, networth]);
       } catch (e: any) {
+        errorThrown = true;
         expect(typeof e).to.equal('object');
-
         expect(e.message).to.include(`cannot store TEXT value in INT column User.id`);
       }
+      expect(errorThrown).to.equal(true);
     });
 
     it('Transaction, auto commit', async () => {
@@ -260,16 +262,6 @@ export function registerBaseTests() {
       expect(res.rows?._array).to.eql([]);
     });
 
-    it('Correctly throws', async () => {
-      const id = chance.string();
-      const { name, age, networth } = generateUserInfo();
-      try {
-        await db.execute('INSERT INTO "User" (id, name, age, networth) VALUES(?, ?, ?, ?)', [id, name, age, networth]);
-      } catch (e: any) {
-        expect(!!e).to.equal(true);
-      }
-    });
-
     it('Rollback', async () => {
       const { id, name, age, networth } = generateUserInfo();
 
@@ -644,5 +636,22 @@ export function registerBaseTests() {
 
       singleConnection.close();
     });
+
+    it('Should attach DBs', async () => {
+      const singleConnection = open('single_connection', { numReadConnections: 0 });
+      await singleConnection.execute('DROP TABLE IF EXISTS Places; ');
+      await singleConnection.execute('CREATE TABLE Places ( id INT PRIMARY KEY, name TEXT NOT NULL) STRICT;');
+      await singleConnection.execute('INSERT INTO "Places" (id, name) VALUES(0, "Beverly Hills")');
+      singleConnection.close();
+
+      db.attach('single_connection', 'another');
+
+      const result = await db.execute('SELECT * from another.Places');
+
+      db.detach('another');
+      QuickSQLite.delete('single_connection');
+
+      expect(result.rows?.length).to.equal(1);
+    });
   });
 }