From fbf6e14a181422ca76b41eb5ee8cb93a1de0d13e Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Wed, 5 Feb 2025 15:08:31 +0200 Subject: [PATCH] Improved connection reference counting Accurate memory access order appropriate for reference counting, just like for shared pointers. Therefore, instead of establishing a global order among threads we can rely on the more light-weight memory access ordering. Also: * Reset the connection pointer after successfully closing the database. * Improved (internal) comments for better understanding. --- dev/connection_holder.h | 35 ++++++++++++++++++++++----------- include/sqlite_orm/sqlite_orm.h | 35 ++++++++++++++++++++++----------- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index 11e6dd0a..0b67f5f8 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -7,43 +7,54 @@ #include "error_code.h" namespace sqlite_orm { - namespace internal { struct connection_holder { - - connection_holder(std::string filename_) : filename(std::move(filename_)) {} + connection_holder(std::string filename) : filename(std::move(filename)) {} void retain() { - if (1 == ++this->_retain_count) { - auto rc = sqlite3_open(this->filename.c_str(), &this->db); - if (rc != SQLITE_OK) { - throw_translated_sqlite_error(db); + // first one opens the connection. + // we presume that the connection is opened once in a single-threaded context [also open forever]. + // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. + if (this->_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { + int rc = sqlite3_open(this->filename.c_str(), &this->db); + if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { + throw_translated_sqlite_error(this->db); } } } void release() { - if (0 == --this->_retain_count) { - auto rc = sqlite3_close(this->db); - if (rc != SQLITE_OK) { - throw_translated_sqlite_error(db); + // last one closes the connection. + // we assume that this might happen by any thread, therefore the counter must serve as a synchronization point. + if (this->_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { + int rc = sqlite3_close(this->db); + if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY { + throw_translated_sqlite_error(this->db); + } else { + this->db = nullptr; } } } sqlite3* get() const { + // note: ensuring a valid DB handle was already memory ordered with `retain()` return this->db; } + /** + * @attention While retrieving the reference count value is atomic it makes only sense at single-threaded points in code. + */ int retain_count() const { - return this->_retain_count; + return this->_retain_count.load(std::memory_order_relaxed); } const std::string filename; protected: sqlite3* db = nullptr; + + private: std::atomic_int _retain_count{}; }; diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index cbd8695b..26b33f73 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -13649,43 +13649,54 @@ namespace sqlite_orm { // #include "error_code.h" namespace sqlite_orm { - namespace internal { struct connection_holder { - - connection_holder(std::string filename_) : filename(std::move(filename_)) {} + connection_holder(std::string filename) : filename(std::move(filename)) {} void retain() { - if (1 == ++this->_retain_count) { - auto rc = sqlite3_open(this->filename.c_str(), &this->db); - if (rc != SQLITE_OK) { - throw_translated_sqlite_error(db); + // first one opens the connection. + // we presume that the connection is opened once in a single-threaded context [also open forever]. + // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. + if (this->_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { + int rc = sqlite3_open(this->filename.c_str(), &this->db); + if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { + throw_translated_sqlite_error(this->db); } } } void release() { - if (0 == --this->_retain_count) { - auto rc = sqlite3_close(this->db); - if (rc != SQLITE_OK) { - throw_translated_sqlite_error(db); + // last one closes the connection. + // we assume that this might happen by any thread, therefore the counter must serve as a synchronization point. + if (this->_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { + int rc = sqlite3_close(this->db); + if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY { + throw_translated_sqlite_error(this->db); + } else { + this->db = nullptr; } } } sqlite3* get() const { + // note: ensuring a valid DB handle was already memory ordered with `retain()` return this->db; } + /** + * @attention While retrieving the reference count value is atomic it makes only sense at single-threaded points in code. + */ int retain_count() const { - return this->_retain_count; + return this->_retain_count.load(std::memory_order_relaxed); } const std::string filename; protected: sqlite3* db = nullptr; + + private: std::atomic_int _retain_count{}; };