Skip to content

Commit

Permalink
changed DM::m_configuration to unordered_map
Browse files Browse the repository at this point in the history
I have discovered very bad bug that caused crashes because of
invalidated references to the configurations caused by QHash after
insertion using DB::addConnection().
std::unordered_map doesn't invalidates references after insertion or
remove 🙌.
  • Loading branch information
silverqx committed Jun 24, 2022
1 parent 98a8e07 commit 735eee5
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
4 changes: 3 additions & 1 deletion include/orm/support/databaseconfiguration.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ TINY_SYSTEM_HEADER

#include <QVariantHash>

#include <unordered_map>

#include "orm/macros/commonnamespace.hpp"
#include "orm/macros/threadlocal.hpp"

Expand All @@ -22,7 +24,7 @@ namespace Orm::Support

public:
/*! Type used for Database Connections map. */
using ConfigurationsType = QHash<QString, QVariantHash>;
using ConfigurationsType = std::unordered_map<QString, QVariantHash>;

/*! Default constructor. */
inline DatabaseConfiguration() = default;
Expand Down
12 changes: 7 additions & 5 deletions src/orm/databasemanager.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "orm/databasemanager.hpp"

#include <range/v3/view/map.hpp>

#include "orm/concerns/hasconnectionresolver.hpp"
#include "orm/exceptions/invalidargumenterror.hpp"
#include "orm/schema.hpp"
Expand Down Expand Up @@ -260,7 +262,7 @@ DatabaseManager::addConnection(const QVariantHash &config, const QString &name)
QStringLiteral("The database connection '%1' already exists.")
.arg(name));

(*m_configuration).insert(name, config);
(*m_configuration).emplace(name, config);

return *this;
}
Expand All @@ -269,7 +271,7 @@ DatabaseManager &
DatabaseManager::addConnections(const ConfigurationsType &configs)
{
for (auto itConfig = configs.cbegin(); itConfig != configs.cend(); ++itConfig)
addConnection(itConfig.value(), itConfig.key());
addConnection(itConfig->second, itConfig->first);

return *this;
}
Expand Down Expand Up @@ -303,7 +305,7 @@ bool DatabaseManager::removeConnection(const QString &name)

// Not connected
if (!(*m_connections).contains(name_)) {
(*m_configuration).remove(name_);
(*m_configuration).erase(name_);
resetDefaultConnection_();
return true;
}
Expand All @@ -321,7 +323,7 @@ bool DatabaseManager::removeConnection(const QString &name)
Schema::m_schemaBuildersCache.erase(name_);

// Remove TinyORM configuration
(*m_configuration).remove(name_);
(*m_configuration).erase(name_);
// Remove Qt's database connection
QSqlDatabase::removeDatabase(name_);

Expand Down Expand Up @@ -364,7 +366,7 @@ QSqlDatabase DatabaseManager::connectEagerly(const QString &name)

QStringList DatabaseManager::connectionNames() const
{
return (*m_configuration).keys();
return (*m_configuration) | ranges::views::keys | ranges::to<QStringList>();
}

QStringList DatabaseManager::openedConnectionNames() const
Expand Down

0 comments on commit 735eee5

Please sign in to comment.