From c3bed1dab8ec429d100d383ad93410d1cfaa16fb Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 11 Apr 2016 19:46:45 -0700 Subject: [core] Fix race condition that could lead to a UNIQUE constraint failure (#4677) --- platform/default/mbgl/storage/offline_database.cpp | 14 +++++++++ platform/default/sqlite3.cpp | 35 ++++++++++++++++++++++ platform/default/sqlite3.hpp | 24 +++++++++++++++ 3 files changed, 73 insertions(+) (limited to 'platform/default') 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; +}; + } } -- cgit v1.2.1