From 56cafee3c47ecc21c6fbff7aad76e6c48052bed9 Mon Sep 17 00:00:00 2001
From: Wander Nauta <info@wandernauta.nl>
Date: Sun, 22 Dec 2024 18:17:04 +0100
Subject: [PATCH] Replace SQLiteWriter mutex with SQLite mutex

The SQLiteWriter commit thread and the user's main thread share the same
connection to the underlying database. The threads are synchronized with
a std::mutex on SQLiteWriter.

However, the SQLite connection already carries its own (recursive) mutex
which we can use instead. This simplifies the design slightly: there can
no longer be a situation where a thread grabs the SQLiteWriter-side lock
but has to wait for the SQLite-side lock, which would be very confusing.

This will not work in builds of SQLite with non-defualt threading modes,
but in such cases sharing a connection among threads is unsafe anyway.
---
 sqlwriter.cc | 16 +++++++++++++---
 sqlwriter.hh |  9 +++++++--
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/sqlwriter.cc b/sqlwriter.cc
index aacde15..cb76425 100644
--- a/sqlwriter.cc
+++ b/sqlwriter.cc
@@ -239,6 +239,16 @@ void MiniSQLite::cycle()
   exec("commit;begin");
 }
 
+MiniSQLite::lock_guard::lock_guard(MiniSQLite& dt) : d_target(dt)
+{
+  sqlite3_mutex_enter(sqlite3_db_mutex(d_target.d_sqlite));
+}
+
+MiniSQLite::lock_guard::~lock_guard()
+{
+  sqlite3_mutex_leave(sqlite3_db_mutex(d_target.d_sqlite));
+}
+
 bool MiniSQLite::haveTable(const string& table)
 {
   return !getSchema(table).empty();
@@ -269,7 +279,7 @@ void SQLiteWriter::commitThread()
   while(!d_pleasequit) {
     usleep(50000);
     if(!(n%20)) {
-      std::lock_guard<std::mutex> lock(d_mutex);
+      MiniSQLite::lock_guard lock(d_db);
       d_db.cycle();
     }
     n++;
@@ -322,7 +332,7 @@ void SQLiteWriter::addValueGeneric(const std::string& table, const T& values, bo
   if(d_flag == SQLWFlag::ReadOnly)
     throw std::runtime_error("Attempting to write to a read-only database instance");
   
-  std::lock_guard<std::mutex> lock(d_mutex);
+  MiniSQLite::lock_guard lock(d_db);
   if(!d_db.isPrepared(table) || d_lastreplace[table] != replace || !equal(values.begin(), values.end(),
                                        d_lastsig[table].cbegin(), d_lastsig[table].cend(),
                             [](const auto& a, const auto& b)
@@ -418,7 +428,7 @@ vector<std::unordered_map<string, MiniSQLite::outvar_t>> SQLiteWriter::queryGen(
   if(msec && d_flag != SQLWFlag::ReadOnly)
     throw std::runtime_error("Timeout only possible for read-only connections");
   
-  std::lock_guard<std::mutex> lock(d_mutex);
+  MiniSQLite::lock_guard lock(d_db);
   d_db.prepare("", q); // we use an empty table name so as not to collide with other things
   int n = 1;
   for(const auto& p : values) {
diff --git a/sqlwriter.hh b/sqlwriter.hh
index a0b062b..dcf8924 100644
--- a/sqlwriter.hh
+++ b/sqlwriter.hh
@@ -3,7 +3,6 @@
 #include <vector>
 #include <unordered_map>
 #include <variant>
-#include <mutex>
 #include <thread>
 #include <iostream>
 #include <map>
@@ -50,6 +49,13 @@ public:
       return iter->second != nullptr;
   }
 
+  struct lock_guard {
+    lock_guard(MiniSQLite&);
+    ~lock_guard();
+  private:
+    MiniSQLite& d_target;
+  };
+
 private:
   sqlite3* d_sqlite;
   std::unordered_map<std::string, sqlite3_stmt*> d_stmts;
@@ -112,7 +118,6 @@ private:
   void commitThread();
   bool d_pleasequit{false};
   std::optional<std::thread> d_thread;
-  std::mutex d_mutex;  
   MiniSQLite d_db;
   SQLWFlag d_flag{SQLWFlag::NoFlag};
   std::unordered_map<std::string, std::vector<std::pair<std::string, std::string>>> d_columns;