From 1e4b0bce79aa6727212137e7a56fd9839af2274b Mon Sep 17 00:00:00 2001 From: silverqx Date: Mon, 29 Jul 2024 14:49:47 +0200 Subject: [PATCH] drivers mysql bugfix loadable MySQL driver --- NOTES.txt | 47 +++++++++++++++++++ .../common/include/orm/drivers/sqldriver.hpp | 3 +- .../drivers/support/sqldriverfactory_p.cpp | 1 + .../include/orm/drivers/mysql/mysqldriver.hpp | 6 +-- .../include/orm/drivers/mysql/mysqlresult.hpp | 2 +- .../src/orm/drivers/mysql/mysqldriver.cpp | 7 ++- .../src/orm/drivers/mysql/mysqlresult.cpp | 8 +++- 7 files changed, 65 insertions(+), 9 deletions(-) diff --git a/NOTES.txt b/NOTES.txt index b3742ddf9..278b75da8 100644 --- a/NOTES.txt +++ b/NOTES.txt @@ -2444,6 +2444,53 @@ which means all the SqlDatabase connection copies and all SqlQuery-ies stop work This is all about how this is designed. +std::weak_ptr - SqlDriver vs MySqlDriver +--------------------------------------------------- +tags: std::enable_shared_from_this +--- + +The std::enable_shared_from_this must be defined as the base class on the SqlDriver +class because the SqlDriver * is returned from the SqlDriver *TinyDriverInstance() in the main.cpp. +The reason why it's so or like this is +the - return std::shared_ptr(std::invoke(createDriverMemFn)) +in the std::shared_ptr SqlDriverFactoryPrivate::createSqlDriverLoadable() +in the sqldriverfactory_p.cpp to be able to correctly populate +the std::enable_shared_from_this::_Wptr. + +If the std::enable_shared_from_this is used and defined as the base class +on the MySqlDriver class, then the +return std::shared_ptr(std::invoke(createDriverMemFn)) will not initialize +the std::enable_shared_from_this::_Wptr correctly because of +the _Can_enable_shared<_Ux> constexpr check +in the template void shared_ptr<_Ux>::_Set_ptr_rep_and_enable_shared(), +which means the std::is_convertible_v<_Yty *, _Yty::_Esft_type *> aka +std::is_convertible_v *> will be false. + +I spent almost whole day on this trying solve it as I wanted to define it on the MySqlDriver and +also wanted to pass the result of the const_cast(*this).weak_from_this() +in MySqlDriver::createResult() as the std::weak_ptr instead of +the std::weak_ptr, but IT'S NOT possible. + +Because of that also the Q_ASSERT(std::dynamic_pointer_cast(driver.lock())); +check exists in the MySqlResult::MySqlResult() constructor and also this constructor +has the std::weak_ptr parameter instead of the std::weak_ptr. +I wanted to have it std::weak_ptr to check the type at compile time, but again +IT'S NOT possible. + +Summary: All of this is not possible because we must return std::shared_ptr() +in the SqlDriverFactoryPrivate::createSqlDriverLoadable() as the SqlDriverFactoryPrivate +doesn't have access to the TinyMySql LOADABLE DLL library what means TinyDrivers doesn't +and can't link against the TinyMySql LOADABLE DLL library, so it knows nothing about +the MySqlDriver type! It only can load it at runtime using LoadLibrary()/dlopen() and will +have access to the SqlDriver interface through a pointer. 😮😮😮😰 + +Also, the MySqlDriver::~MySqlDriver() can' be inline because of the TinyMySql loadable module, +to destroy the MySqlDriver instance from the same DLL where it was initially instantiated. +It would also work as the inline method, but it could make problems if eg. TinyDrivers DLL +and TinyMySql DLL would be compiled with different compiler versions, or in some edge cases +when the memory manager would be different for both DLL libraries. + + MySQL C connector - invoked functions: -------------------------------------- diff --git a/drivers/common/include/orm/drivers/sqldriver.hpp b/drivers/common/include/orm/drivers/sqldriver.hpp index adaae90f7..78b24a175 100644 --- a/drivers/common/include/orm/drivers/sqldriver.hpp +++ b/drivers/common/include/orm/drivers/sqldriver.hpp @@ -24,7 +24,8 @@ namespace Orm::Drivers class SqlResult; /*! Database driver abstract class. */ - class TINYDRIVERS_EXPORT SqlDriver : public Concerns::SelectsAllColumnsWithLimit0 + class TINYDRIVERS_EXPORT SqlDriver : public Concerns::SelectsAllColumnsWithLimit0, + public std::enable_shared_from_this // See NOTES.txt[std::enable_shared_from_this] { Q_DISABLE_COPY_MOVE(SqlDriver) Q_DECLARE_PRIVATE(SqlDriver) // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) diff --git a/drivers/common/src/orm/drivers/support/sqldriverfactory_p.cpp b/drivers/common/src/orm/drivers/support/sqldriverfactory_p.cpp index 50ce074d5..f0341773e 100644 --- a/drivers/common/src/orm/drivers/support/sqldriverfactory_p.cpp +++ b/drivers/common/src/orm/drivers/support/sqldriverfactory_p.cpp @@ -237,6 +237,7 @@ SqlDriverFactoryPrivate::createSqlDriverLoadable(const QString &driverBasenameRa // This should never happen :/, it's checked earlier in loadSqlDriverCommon() Q_ASSERT(createDriverMemFn); + // See NOTES.txt[std::enable_shared_from_this] for more info return std::shared_ptr(std::invoke(createDriverMemFn)); } diff --git a/drivers/mysql/include/orm/drivers/mysql/mysqldriver.hpp b/drivers/mysql/include/orm/drivers/mysql/mysqldriver.hpp index b575ba962..19766283e 100644 --- a/drivers/mysql/include/orm/drivers/mysql/mysqldriver.hpp +++ b/drivers/mysql/include/orm/drivers/mysql/mysqldriver.hpp @@ -16,9 +16,7 @@ namespace Orm::Drivers::MySql class MySqlResultPrivate; /*! MySQL database driver. */ - class TINYDRIVERS_EXPORT MySqlDriver final : - public SqlDriver, - public std::enable_shared_from_this + class TINYDRIVERS_EXPORT MySqlDriver final : public SqlDriver { Q_DISABLE_COPY_MOVE(MySqlDriver) Q_DECLARE_PRIVATE(MySqlDriver) // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) @@ -32,7 +30,7 @@ namespace Orm::Drivers::MySql /*! Default constructor. */ MySqlDriver(); /*! Virtual destructor. */ - ~MySqlDriver() final = default; + ~MySqlDriver() final; /*! Open the database connection using the given values. */ bool open(const QString &database, const QString &username, diff --git a/drivers/mysql/include/orm/drivers/mysql/mysqlresult.hpp b/drivers/mysql/include/orm/drivers/mysql/mysqlresult.hpp index 27c632df9..493888013 100644 --- a/drivers/mysql/include/orm/drivers/mysql/mysqlresult.hpp +++ b/drivers/mysql/include/orm/drivers/mysql/mysqlresult.hpp @@ -23,7 +23,7 @@ namespace Orm::Drivers::MySql public: /*! Constructor. */ - explicit MySqlResult(const std::weak_ptr &driver); + explicit MySqlResult(const std::weak_ptr &driver); /*! Virtual destructor. */ ~MySqlResult() noexcept final; diff --git a/drivers/mysql/src/orm/drivers/mysql/mysqldriver.cpp b/drivers/mysql/src/orm/drivers/mysql/mysqldriver.cpp index 0b51ae3fb..86c4395de 100644 --- a/drivers/mysql/src/orm/drivers/mysql/mysqldriver.cpp +++ b/drivers/mysql/src/orm/drivers/mysql/mysqldriver.cpp @@ -29,6 +29,10 @@ MySqlDriver::MySqlDriver() : SqlDriver(std::make_unique()) {} +/* Can' be inline because of the TinyMySql loadable module, to destroy the MySqlDriver + instance from the same DLL where it was initially instantiated. */ +MySqlDriver::~MySqlDriver() = default; + bool MySqlDriver::open( const QString &database, const QString &username, const QString &password, const QString &host, const int port, const QString &options) @@ -226,7 +230,8 @@ std::unique_ptr MySqlDriver::createResult() const /* We must use the const_cast<> as the weak_from_this() return type is controlled by the current method const-nes, what means it's only our implementation detail that we have to use the const_cast<> as we can't control this. Also, it's better - to have the same const-nes for the createResult() as defined in the QtSql. */ + to have the same const-nes for the createResult() as defined in the QtSql. + See NOTES.txt[std::enable_shared_from_this] for more info. */ return std::make_unique( const_cast(*this).weak_from_this()); // NOLINT(cppcoreguidelines-pro-type-const-cast) } diff --git a/drivers/mysql/src/orm/drivers/mysql/mysqlresult.cpp b/drivers/mysql/src/orm/drivers/mysql/mysqlresult.cpp index 886b51630..64c67c527 100644 --- a/drivers/mysql/src/orm/drivers/mysql/mysqlresult.cpp +++ b/drivers/mysql/src/orm/drivers/mysql/mysqlresult.cpp @@ -20,9 +20,13 @@ namespace Orm::Drivers::MySql /* public */ -MySqlResult::MySqlResult(const std::weak_ptr &driver) +MySqlResult::MySqlResult(const std::weak_ptr &driver) : SqlResult(std::make_unique(driver)) -{} +{ + /* Check an empty shared_ptr<> using the std::shared_ptr::operator bool(). + See NOTES.txt[std::enable_shared_from_this] for more info. */ + Q_ASSERT(std::dynamic_pointer_cast(driver.lock())); +} MySqlResult::~MySqlResult() noexcept {