cleanup transaction hooks on close

Author: stevensJourney

cpp/ConnectionPool.cpp

@@ -19,6 +19,7 @@ ConnectionPool::ConnectionPool(std::string dbName, std::string docPath,
 
   onContextCallback = nullptr;
   isConcurrencyEnabled = maxReads > 0;
+  isClosed = false;
 
   readConnections = new ConnectionState *[maxReads];
   // Open the read connections
@@ -169,9 +170,8 @@ void ConnectionPool::closeContext(ConnectionLockId contextId) {
 }
 
 void ConnectionPool::closeAll() {
-  // Prevent this pointer from being used if closed
-  rollbackPayload.dbName = NULL;
-  commitPayload.dbName = NULL;
+  isClosed = true;
+  // Stop any callbacks
   sqlite3_commit_hook(writeConnection.connection,
                       NULL, NULL);
   sqlite3_rollback_hook(writeConnection.connection, 

cpp/ConnectionPool.h

@@ -66,15 +66,17 @@ class ConnectionPool {
   std::vector<ConnectionLockId> writeQueue;
 
   // Cached constant payloads for c style commit/rollback callbacks
-  TransactionCallbackPayload commitPayload;
-  TransactionCallbackPayload rollbackPayload;
+  const TransactionCallbackPayload commitPayload;
+  const TransactionCallbackPayload rollbackPayload;
 
   void (*onContextCallback)(std::string, ConnectionLockId);
   void (*onCommitCallback)(const TransactionCallbackPayload *);
 
   bool isConcurrencyEnabled;
 
 public:
+  bool isClosed;
+
   ConnectionPool(std::string dbName, std::string docPath,
                  unsigned int numReadConnections);
   ~ConnectionPool();

cpp/ConnectionState.cpp

@@ -59,7 +59,6 @@ std::future<void> ConnectionState::refreshSchema() {
 
 void ConnectionState::close() {
   {
-    // Now signal the thread to stop and notify it
     std::unique_lock<std::mutex> g(workQueueMutex);
     // prevent any new work from being queued 
     isClosed = true;

cpp/bindings.cpp

@@ -77,19 +77,25 @@ void transactionFinalizerHandler(const TransactionCallbackPayload *payload) {
    * This function triggers an async invocation to call watch callbacks,
    * avoiding holding SQLite up.
    */
-  invoker->invokeAsync([payload] {
+
+  // Make a copy of the payload data, this avoids a potential race condition
+  // where the async invocation might occur after closing a connection
+  auto dbName = std::make_shared<std::string>(*payload->dbName);
+  int event = payload->event;
+  invoker->invokeAsync([dbName, event] {
     try {
-      // Prevent trying to create a JSI string for a potentially closed DB
-      if (payload == NULL || payload->dbName == NULL) {
+     
+     ConnectionPool* connection = getConnection(*dbName);
+      if (connection == nullptr || connection->isClosed) {
         return;
       }
-      
+
       auto global = runtime->global();
       jsi::Function handlerFunction = global.getPropertyAsFunction(
           *runtime, "triggerTransactionFinalizerHook");
 
-      auto jsiDbName = jsi::String::createFromAscii(*runtime, *payload->dbName);
-      auto jsiEventType = jsi::Value((int)payload->event);
+      auto jsiDbName = jsi::String::createFromAscii(*runtime, *dbName);
+      auto jsiEventType = jsi::Value(event);
       handlerFunction.call(*runtime, move(jsiDbName), move(jsiEventType));
     } catch (jsi::JSINativeException e) {
       std::cout << e.what() << std::endl;

cpp/sqliteBridge.cpp

@@ -32,6 +32,16 @@ SQLiteOPResult generateNotOpenResult(std::string const &dbName) {
   };
 }
 
+ConnectionPool *getConnection(std::string const dbName) {
+  if (dbMap.count(dbName) == 0) {
+    // Connection is already closed
+    return nullptr;
+  }
+
+  return dbMap[dbName];
+}
+
+
 /**
  * Opens SQL database with default settings
  */

cpp/sqliteBridge.h

@@ -40,6 +40,8 @@ SQLiteOPResult sqliteCloseDb(string const dbName);
 
 void sqliteCloseAll();
 
+ConnectionPool *getConnection(std::string const dbName);
+
 SQLiteOPResult sqliteRemoveDb(string const dbName, string const docPath);
 
 /**