[core] Fix race condition that could lead to a UNIQUE constraint failure (#4677)

3 files changed, 73 insertions, 0 deletions

diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp
index 0b8dec01bf..44d0837321 100644
--- a/platform/default/mbgl/storage/offline_database.cpp
+++ b/platform/default/mbgl/storage/offline_database.cpp
@@ -239,6 +239,10 @@ bool OfflineDatabase::putResource(const Resource& resource,
 
     // We can't use REPLACE because it would change the id value.
 
+    // Begin an immediate-mode transaction to ensure that two writers do not attempt
+    // to INSERT a resource at the same moment.
+    Transaction transaction(*db, Transaction::Immediate);
+
     Statement update = getStatement(
         "UPDATE resources "
         "SET kind       = ?1, "
@@ -267,6 +271,7 @@ bool OfflineDatabase::putResource(const Resource& resource,
 
     update->run();
     if (db->changes() != 0) {
+        transaction.commit();
         return false;
     }
 
@@ -290,6 +295,8 @@ bool OfflineDatabase::putResource(const Resource& resource,
     }
 
     insert->run();
+    transaction.commit();
+
     return true;
 }
 
@@ -380,6 +387,10 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile,
 
     // We can't use REPLACE because it would change the id value.
 
+    // Begin an immediate-mode transaction to ensure that two writers do not attempt
+    // to INSERT a resource at the same moment.
+    Transaction transaction(*db, Transaction::Immediate);
+
     Statement update = getStatement(
         "UPDATE tiles "
         "SET modified       = ?1, "
@@ -414,6 +425,7 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile,
 
     update->run();
     if (db->changes() != 0) {
+        transaction.commit();
         return false;
     }
 
@@ -440,6 +452,8 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile,
     }
 
     insert->run();
+    transaction.commit();
+
     return true;
 }
 
diff --git a/platform/default/sqlite3.cpp b/platform/default/sqlite3.cpp
index bf0bbfc683..a5d619406f 100644
--- a/platform/default/sqlite3.cpp
+++ b/platform/default/sqlite3.cpp
@@ -338,5 +338,40 @@ void Statement::clearBindings() {
     sqlite3_clear_bindings(stmt);
 }
 
+Transaction::Transaction(Database& db_, Mode mode)
+    : db(db_) {
+    switch (mode) {
+    case Deferred:
+        db.exec("BEGIN DEFERRED TRANSACTION");
+        break;
+    case Immediate:
+        db.exec("BEGIN IMMEDIATE TRANSACTION");
+        break;
+    case Exclusive:
+        db.exec("BEGIN EXCLUSIVE TRANSACTION");
+        break;
+    }
+}
+
+Transaction::~Transaction() {
+    if (needRollback) {
+        try {
+            rollback();
+        } catch (...) {
+            // Ignore failed rollbacks in destructor.
+        }
+    }
+}
+
+void Transaction::commit() {
+    needRollback = false;
+    db.exec("COMMIT TRANSACTION");
+}
+
+void Transaction::rollback() {
+    needRollback = false;
+    db.exec("ROLLBACK TRANSACTION");
+}
+
 } // namespace sqlite
 } // namespace mapbox
diff --git a/platform/default/sqlite3.hpp b/platform/default/sqlite3.hpp
index abe83a2d44..57ee18e9f3 100644
--- a/platform/default/sqlite3.hpp
+++ b/platform/default/sqlite3.hpp
@@ -88,5 +88,29 @@ private:
     sqlite3_stmt *stmt = nullptr;
 };
 
+class Transaction {
+private:
+    Transaction(const Transaction&) = delete;
+    Transaction(Transaction&&) = delete;
+    Transaction& operator=(const Transaction&) = delete;
+
+public:
+    enum Mode {
+        Deferred,
+        Immediate,
+        Exclusive
+    };
+
+    Transaction(Database&, Mode = Deferred);
+    ~Transaction();
+
+    void commit();
+    void rollback();
+
+private:
+    Database& db;
+    bool needRollback = true;
+};
+
 }
 }