Skip to content

Commit

Permalink
drivers mysql bugfix loadable MySQL driver
Browse files Browse the repository at this point in the history
  • Loading branch information
silverqx committed Jul 29, 2024
1 parent c49fe45 commit 1e4b0bc
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 9 deletions.
47 changes: 47 additions & 0 deletions NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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> - SqlDriver vs MySqlDriver
---------------------------------------------------
tags: std::enable_shared_from_this<SqlDriver>
---

The std::enable_shared_from_this<SqlDriver> 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<SqlDriver>(std::invoke(createDriverMemFn))
in the std::shared_ptr<SqlDriver> SqlDriverFactoryPrivate::createSqlDriverLoadable()
in the sqldriverfactory_p.cpp to be able to correctly populate
the std::enable_shared_from_this<SqlDriver>::_Wptr.

If the std::enable_shared_from_this<MySqlDriver> is used and defined as the base class
on the MySqlDriver class, then the
return std::shared_ptr<SqlDriver>(std::invoke(createDriverMemFn)) will not initialize
the std::enable_shared_from_this<MySqlDriver>::_Wptr correctly because of
the _Can_enable_shared<_Ux> constexpr check
in the template <class _Ux> 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<SqlDriver *, std::enable_shared_from_this<MySqlDriver> *> 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<MySqlDriver &>(*this).weak_from_this()
in MySqlDriver::createResult() as the std::weak_ptr<MySqlDriver> instead of
the std::weak_ptr<SqlDriver>, but IT'S NOT possible.

Because of that also the Q_ASSERT(std::dynamic_pointer_cast<MySqlDriver>(driver.lock()));
check exists in the MySqlResult::MySqlResult() constructor and also this constructor
has the std::weak_ptr<SqlDriver> parameter instead of the std::weak_ptr<MySqlDriver>.
I wanted to have it std::weak_ptr<MySqlDriver> 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<SqlDriver>()
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:
--------------------------------------

Expand Down
3 changes: 2 additions & 1 deletion drivers/common/include/orm/drivers/sqldriver.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<SqlDriver> // See NOTES.txt[std::enable_shared_from_this<SqlDriver>]
{
Q_DISABLE_COPY_MOVE(SqlDriver)
Q_DECLARE_PRIVATE(SqlDriver) // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<SqlDriver>] for more info
return std::shared_ptr<SqlDriver>(std::invoke(createDriverMemFn));
}

Expand Down
6 changes: 2 additions & 4 deletions drivers/mysql/include/orm/drivers/mysql/mysqldriver.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<MySqlDriver>
class TINYDRIVERS_EXPORT MySqlDriver final : public SqlDriver
{
Q_DISABLE_COPY_MOVE(MySqlDriver)
Q_DECLARE_PRIVATE(MySqlDriver) // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion drivers/mysql/include/orm/drivers/mysql/mysqlresult.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace Orm::Drivers::MySql

public:
/*! Constructor. */
explicit MySqlResult(const std::weak_ptr<MySqlDriver> &driver);
explicit MySqlResult(const std::weak_ptr<SqlDriver> &driver);
/*! Virtual destructor. */
~MySqlResult() noexcept final;

Expand Down
7 changes: 6 additions & 1 deletion drivers/mysql/src/orm/drivers/mysql/mysqldriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ MySqlDriver::MySqlDriver()
: SqlDriver(std::make_unique<MySqlDriverPrivate>())
{}

/* 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)
Expand Down Expand Up @@ -226,7 +230,8 @@ std::unique_ptr<SqlResult> 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<SqlDriver>] for more info. */
return std::make_unique<MySqlResult>(
const_cast<MySqlDriver &>(*this).weak_from_this()); // NOLINT(cppcoreguidelines-pro-type-const-cast)
}
Expand Down
8 changes: 6 additions & 2 deletions drivers/mysql/src/orm/drivers/mysql/mysqlresult.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,13 @@ namespace Orm::Drivers::MySql

/* public */

MySqlResult::MySqlResult(const std::weak_ptr<MySqlDriver> &driver)
MySqlResult::MySqlResult(const std::weak_ptr<SqlDriver> &driver)
: SqlResult(std::make_unique<MySqlResultPrivate>(driver))
{}
{
/* Check an empty shared_ptr<> using the std::shared_ptr<T>::operator bool().
See NOTES.txt[std::enable_shared_from_this<SqlDriver>] for more info. */
Q_ASSERT(std::dynamic_pointer_cast<MySqlDriver>(driver.lock()));
}

MySqlResult::~MySqlResult() noexcept
{
Expand Down

0 comments on commit 1e4b0bc

Please sign in to comment.