From 0f37daaa3036ec7cfc52e11389cc2a495a554205 Mon Sep 17 00:00:00 2001 From: emeric Date: Fri, 15 Mar 2024 16:26:59 +0100 Subject: [PATCH] Completely removed the transaction checker in release (which was here but as a noop checker) --- src/libs/database/CMakeLists.txt | 5 +++- src/libs/database/impl/Session.cpp | 11 ++++++++- src/libs/database/impl/TransactionChecker.cpp | 24 +++++-------------- src/libs/database/include/database/Object.hpp | 11 ++++++++- .../database/include/database/Session.hpp | 16 ++++++++++--- .../include/database/TransactionChecker.hpp | 12 +++++++++- 6 files changed, 54 insertions(+), 25 deletions(-) diff --git a/src/libs/database/CMakeLists.txt b/src/libs/database/CMakeLists.txt index b7ed139a..30fc9a7c 100644 --- a/src/libs/database/CMakeLists.txt +++ b/src/libs/database/CMakeLists.txt @@ -18,12 +18,15 @@ add_library(lmsdatabase SHARED impl/SqlQuery.cpp impl/Track.cpp impl/TrackBookmark.cpp - impl/TransactionChecker.cpp impl/Types.cpp impl/User.cpp impl/Utils.cpp ) +if (CMAKE_BUILD_TYPE MATCHES "Debug") +target_sources(lmsdatabase PRIVATE impl/TransactionChecker.cpp) +endif() + target_include_directories(lmsdatabase INTERFACE include ) diff --git a/src/libs/database/impl/Session.cpp b/src/libs/database/impl/Session.cpp index 58289236..30754d48 100644 --- a/src/libs/database/impl/Session.cpp +++ b/src/libs/database/impl/Session.cpp @@ -53,26 +53,34 @@ namespace lms::db : _lock{ mutex }, _transaction{ session } { +#if LMS_CHECK_TRANSACTION_ACCESSES TransactionChecker::pushWriteTransaction(_transaction.session()); +#endif } WriteTransaction::~WriteTransaction() { +#if LMS_CHECK_TRANSACTION_ACCESSES TransactionChecker::popWriteTransaction(_transaction.session()); +#endif - core::tracing::ScopedTrace _trace{ "Database", core::tracing::Level::Detailed, "CommitWriteTransaction" }; + core::tracing::ScopedTrace _trace{ "Database", core::tracing::Level::Detailed, "Commit" }; _transaction.commit(); } ReadTransaction::ReadTransaction(Wt::Dbo::Session& session) : _transaction{ session } { +#if LMS_CHECK_TRANSACTION_ACCESSES TransactionChecker::pushReadTransaction(_transaction.session()); +#endif } ReadTransaction::~ReadTransaction() { +#if LMS_CHECK_TRANSACTION_ACCESSES TransactionChecker::popReadTransaction(_transaction.session()); +#endif } Session::Session(Db& db) @@ -119,6 +127,7 @@ namespace lms::db // Initial creation case try { + auto transaction{ createWriteTransaction() }; _session.createTables(); LMS_LOG(DB, INFO, "Tables created"); } diff --git a/src/libs/database/impl/TransactionChecker.cpp b/src/libs/database/impl/TransactionChecker.cpp index 315c953b..9ab9c98f 100644 --- a/src/libs/database/impl/TransactionChecker.cpp +++ b/src/libs/database/impl/TransactionChecker.cpp @@ -19,19 +19,13 @@ #include "database/TransactionChecker.hpp" +static_assert(LMS_CHECK_TRANSACTION_ACCESSES, "File should be excluded from build"); + #include - #include "database/Session.hpp" -#if !defined(NDEBUG) -#define LMS_CHECK_TRANSACTION_ACCESSES 1 -#else -#define LMS_CHECK_TRANSACTION_ACCESSES 0 -#endif - namespace lms::db { -#if LMS_CHECK_TRANSACTION_ACCESSES namespace { struct StackEntry @@ -42,7 +36,6 @@ namespace lms::db static thread_local std::vector transactionStack; } -#endif void TransactionChecker::pushWriteTransaction(Wt::Dbo::Session& session) { @@ -64,26 +57,21 @@ namespace lms::db popTransaction(TransactionType::Read, session); } - void TransactionChecker::pushTransaction([[maybe_unused]] TransactionType type, [[maybe_unused]] Wt::Dbo::Session& session) + void TransactionChecker::pushTransaction(TransactionType type, Wt::Dbo::Session& session) { -#if LMS_CHECK_TRANSACTION_ACCESSES assert(transactionStack.empty() || transactionStack.back().session == &session); transactionStack.push_back(StackEntry{ type, &session }); -#endif // LMS_CHECK_TRANSACTION_ACCESSES } - void TransactionChecker::popTransaction([[maybe_unused]] TransactionType type, [[maybe_unused]] Wt::Dbo::Session& session) + void TransactionChecker::popTransaction(TransactionType type, Wt::Dbo::Session& session) { -#if LMS_CHECK_TRANSACTION_ACCESSES - assert(!transactionStack.empty()); assert(transactionStack.back().type == type); assert(transactionStack.back().session == &session); transactionStack.pop_back(); -#endif // LMS_CHECK_TRANSACTION_ACCESSES } - void TransactionChecker::checkWriteTransaction([[maybe_unused]] Wt::Dbo::Session& session) + void TransactionChecker::checkWriteTransaction(Wt::Dbo::Session& session) { assert(!transactionStack.empty()); assert(transactionStack.back().type == TransactionType::Write); @@ -95,7 +83,7 @@ namespace lms::db checkWriteTransaction(session.getDboSession()); } - void TransactionChecker::checkReadTransaction([[maybe_unused]] Wt::Dbo::Session& session) + void TransactionChecker::checkReadTransaction(Wt::Dbo::Session& session) { assert(!transactionStack.empty()); assert(transactionStack.back().session == &session); diff --git a/src/libs/database/include/database/Object.hpp b/src/libs/database/include/database/Object.hpp index 6f37a4df..3c7452b5 100644 --- a/src/libs/database/include/database/Object.hpp +++ b/src/libs/database/include/database/Object.hpp @@ -39,10 +39,19 @@ namespace lms::db bool operator==(const ObjectPtr& other) const { return _obj == other._obj; } bool operator!=(const ObjectPtr& other) const { return other._obj != _obj; } - auto modify() { TransactionChecker::checkWriteTransaction(*_obj.session()); return _obj.modify(); } + auto modify() + { +#if LMS_CHECK_TRANSACTION_ACCESSES + TransactionChecker::checkWriteTransaction(*_obj.session()); +#endif + return _obj.modify(); + } + void remove() { +#if LMS_CHECK_TRANSACTION_ACCESSES TransactionChecker::checkWriteTransaction(*_obj.session()); +#endif if (_obj->hasOnPreRemove()) _obj.modify()->onPreRemove(); diff --git a/src/libs/database/include/database/Session.hpp b/src/libs/database/include/database/Session.hpp index c0175e99..544ff975 100644 --- a/src/libs/database/include/database/Session.hpp +++ b/src/libs/database/include/database/Session.hpp @@ -71,8 +71,18 @@ namespace lms::db [[nodiscard]] WriteTransaction createWriteTransaction(); [[nodiscard]] ReadTransaction createReadTransaction(); - void checkWriteTransaction() { TransactionChecker::checkWriteTransaction(_session); } - void checkReadTransaction() { TransactionChecker::checkReadTransaction(_session); } + void checkWriteTransaction() + { +#if LMS_CHECK_TRANSACTION_ACCESSES + TransactionChecker::checkWriteTransaction(_session); +#endif + } + void checkReadTransaction() + { +#if LMS_CHECK_TRANSACTION_ACCESSES + TransactionChecker::checkReadTransaction(_session); +#endif + } void analyze(); void optimize(); @@ -85,7 +95,7 @@ namespace lms::db template typename Object::pointer create(Args&&... args) { - TransactionChecker::checkWriteTransaction(_session); + checkWriteTransaction(); typename Object::pointer res{ Object::create(*this, std::forward(args)...) }; getDboSession().flush(); diff --git a/src/libs/database/include/database/TransactionChecker.hpp b/src/libs/database/include/database/TransactionChecker.hpp index 5fe92510..58128e00 100644 --- a/src/libs/database/include/database/TransactionChecker.hpp +++ b/src/libs/database/include/database/TransactionChecker.hpp @@ -19,6 +19,14 @@ #pragma once +#if !defined(NDEBUG) +#define LMS_CHECK_TRANSACTION_ACCESSES 1 +#else +#define LMS_CHECK_TRANSACTION_ACCESSES 0 +#endif + +#if LMS_CHECK_TRANSACTION_ACCESSES + #include #include @@ -50,4 +58,6 @@ namespace lms::db static void pushTransaction(TransactionType type, Wt::Dbo::Session& session); static void popTransaction(TransactionType type, Wt::Dbo::Session& session); }; -} \ No newline at end of file +} + +#endif \ No newline at end of file