feedc0de/qt-creator
1
0
Fork 0
forked from qt-creator/qt-creator
Code Pull Requests Activity

Remove code that tried to handle SshConnections in non-main threads

Browse Source 
The SshConnection wasn't used in non-main thread anyway and the
implementation couldn't work properly in multiple threads.

Fix "acquire" typo in variable and class names.

Make SshConnectionManager a child of application object, so that
it will be deleted together with it.

Replace forever with ranged for loop.

Change-Id: I3c3592eb5ce20b463d5a656f94cadaccccf652dc
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
Reviewed-by: hjk <hjk@qt.io>
This commit is contained in:
Jarek Kobus
2021-09-09 15:29:28 +02:00
parent 781031be9c
commit 2bdbaaa2fd
1 changed files with 34 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files

112
src/libs/ssh/sshconnectionmanager.cpp
View File

@@ -32,36 +32,34 @@
#include <QCoreApplication> #include <QCoreApplication>
#include <QList> #include <QList>
#include <QMutex>
#include <QMutexLocker>
#include <QObject> #include <QObject>
#include <QThread> #include <QThread>
#include <QTimer> #include <QTimer>
namespace QSsh { namespace QSsh {
namespace Internal { namespace Internal {
class UnaquiredConnection { class UnacquiredConnection {
public: public:
UnaquiredConnection(SshConnection *conn) : connection(conn), scheduledForRemoval(false) {} UnacquiredConnection(SshConnection *conn) : connection(conn), scheduledForRemoval(false) {}
SshConnection *connection; SshConnection *connection;
bool scheduledForRemoval; bool scheduledForRemoval;
}; };
bool operator==(const UnaquiredConnection &c1, const UnaquiredConnection &c2) { bool operator==(const UnacquiredConnection &c1, const UnacquiredConnection &c2) {
return c1.connection == c2.connection; return c1.connection == c2.connection;
} }
bool operator!=(const UnaquiredConnection &c1, const UnaquiredConnection &c2) { bool operator!=(const UnacquiredConnection &c1, const UnacquiredConnection &c2) {
return !(c1 == c2); return !(c1 == c2);
} }
static class SshConnectionManager *s_instance = nullptr;
class SshConnectionManager : public QObject class SshConnectionManager : public QObject
{ {
Q_OBJECT
public: public:
SshConnectionManager() SshConnectionManager(QObject *parent)
: QObject(parent)
{ {
moveToThread(QCoreApplication::instance()->thread());
connect(&m_removalTimer, &QTimer::timeout, connect(&m_removalTimer, &QTimer::timeout,
this, &SshConnectionManager::removeInactiveConnections); this, &SshConnectionManager::removeInactiveConnections);
m_removalTimer.start(SshSettings::connectionSharingTimeout() * 1000 * 60 / 2); m_removalTimer.start(SshSettings::connectionSharingTimeout() * 1000 * 60 / 2);
@@ -69,27 +67,24 @@ public:
~SshConnectionManager() ~SshConnectionManager()
{ {
foreach (const UnaquiredConnection &connection, m_unacquiredConnections) { for (const UnacquiredConnection &connection : qAsConst(m_unacquiredConnections)) {
disconnect(connection.connection, nullptr, this, nullptr); disconnect(connection.connection, nullptr, this, nullptr);
delete connection.connection; delete connection.connection;
} }
QTC_CHECK(m_acquiredConnections.isEmpty()); QTC_CHECK(m_acquiredConnections.isEmpty());
QTC_CHECK(m_deprecatedConnections.isEmpty()); QTC_CHECK(m_deprecatedConnections.isEmpty());
s_instance = nullptr;
} }
SshConnection *acquireConnection(const SshConnectionParameters &sshParams) SshConnection *acquireConnection(const SshConnectionParameters &sshParams)
{ {
QMutexLocker locker(&m_listMutex);
// Check in-use connections: // Check in-use connections:
foreach (SshConnection * const connection, m_acquiredConnections) { for (SshConnection * const connection : qAsConst(m_acquiredConnections)) {
if (connection->connectionParameters() != sshParams) if (connection->connectionParameters() != sshParams)
continue; continue;
if (connection->thread() != QThread::currentThread())
continue;
if (connection->sharingEnabled() != SshSettings::connectionSharingEnabled()) if (connection->sharingEnabled() != SshSettings::connectionSharingEnabled())
continue; continue;
@@ -101,20 +96,13 @@ public:
} }
// Check cached open connections: // Check cached open connections:
foreach (const UnaquiredConnection &c, m_unacquiredConnections) { for (int i = 0; i < m_unacquiredConnections.count(); ++i) {
SshConnection * const connection = c.connection; SshConnection * const connection = m_unacquiredConnections.at(i).connection;
if (connection->state() != SshConnection::Connected if (connection->state() != SshConnection::Connected
|| connection->connectionParameters() != sshParams) || connection->connectionParameters() != sshParams)
continue; continue;
auto currentThread = QThread::currentThread(); m_unacquiredConnections.removeAt(i);
if (connection->thread() != currentThread) {
QMetaObject::invokeMethod(this, [this, connection, currentThread] {
switchToCallerThread(connection, currentThread);
}, Qt::BlockingQueuedConnection);
}
m_unacquiredConnections.removeOne(c);
m_acquiredConnections.append(connection); m_acquiredConnections.append(connection);
return connection; return connection;
} }
@@ -131,37 +119,21 @@ public:
void releaseConnection(SshConnection *connection) void releaseConnection(SshConnection *connection)
{ {
QMutexLocker locker(&m_listMutex); const bool wasAcquired = m_acquiredConnections.removeOne(connection);
QTC_ASSERT(wasAcquired == connection->sharingEnabled(), return);
const bool wasAquired = m_acquiredConnections.removeOne(connection);
QTC_ASSERT(wasAquired == connection->sharingEnabled(), return);
if (m_acquiredConnections.contains(connection)) if (m_acquiredConnections.contains(connection))
return; return;
bool doDelete = false; bool doDelete = false;
connection->moveToThread(QCoreApplication::instance()->thread());
if (!connection->sharingEnabled()) { if (!connection->sharingEnabled()) {
doDelete = true; doDelete = true;
} else if (m_deprecatedConnections.removeOne(connection) } else if (m_deprecatedConnections.removeOne(connection)
|| connection->state() != SshConnection::Connected) { || connection->state() != SshConnection::Connected) {
doDelete = true; doDelete = true;
} else { } else {
QTC_ASSERT(!m_unacquiredConnections.contains(UnaquiredConnection(connection)), return); UnacquiredConnection uc(connection);
QTC_ASSERT(!m_unacquiredConnections.contains(uc), return);
// It can happen that two or more connections with the same parameters were acquired m_unacquiredConnections.append(uc);
// if the clients were running in different threads. Only keep one of them in
// such a case.
bool haveConnection = false;
foreach (const UnaquiredConnection &c, m_unacquiredConnections) {
if (c.connection->connectionParameters() == connection->connectionParameters()) {
haveConnection = true;
break;
}
}
if (!haveConnection)
m_unacquiredConnections.append(UnaquiredConnection(connection));
else
doDelete = true;
} }
if (doDelete) { if (doDelete) {
@@ -173,8 +145,6 @@ public:
void forceNewConnection(const SshConnectionParameters &sshParams) void forceNewConnection(const SshConnectionParameters &sshParams)
{ {
QMutexLocker locker(&m_listMutex);
for (int i = 0; i < m_unacquiredConnections.count(); ++i) { for (int i = 0; i < m_unacquiredConnections.count(); ++i) {
SshConnection * const connection = m_unacquiredConnections.at(i).connection; SshConnection * const connection = m_unacquiredConnections.at(i).connection;
if (connection->connectionParameters() == sshParams) { if (connection->connectionParameters() == sshParams) {
@@ -185,7 +155,7 @@ public:
} }
} }
foreach (SshConnection * const connection, m_acquiredConnections) { for (SshConnection * const connection : qAsConst(m_acquiredConnections)) {
if (connection->connectionParameters() == sshParams) { if (connection->connectionParameters() == sshParams) {
if (!m_deprecatedConnections.contains(connection)) if (!m_deprecatedConnections.contains(connection))
m_deprecatedConnections.append(connection); m_deprecatedConnections.append(connection);
@@ -194,20 +164,13 @@ public:
} }
private: private:
Q_INVOKABLE void switchToCallerThread(SshConnection *connection, QObject *threadObj)
{
connection->moveToThread(qobject_cast<QThread *>(threadObj));
}
void cleanup() void cleanup()
{ {
QMutexLocker locker(&m_listMutex);
SshConnection *currentConnection = qobject_cast<SshConnection *>(sender()); SshConnection *currentConnection = qobject_cast<SshConnection *>(sender());
if (!currentConnection) if (!currentConnection)
return; return;
if (m_unacquiredConnections.removeOne(UnaquiredConnection(currentConnection))) { if (m_unacquiredConnections.removeOne(UnacquiredConnection(currentConnection))) {
disconnect(currentConnection, nullptr, this, nullptr); disconnect(currentConnection, nullptr, this, nullptr);
currentConnection->deleteLater(); currentConnection->deleteLater();
} }
@@ -215,9 +178,8 @@ private:
void removeInactiveConnections() void removeInactiveConnections()
{ {
QMutexLocker locker(&m_listMutex);
for (int i = m_unacquiredConnections.count() - 1; i >= 0; --i) { for (int i = m_unacquiredConnections.count() - 1; i >= 0; --i) {
UnaquiredConnection &c = m_unacquiredConnections[i]; UnacquiredConnection &c = m_unacquiredConnections[i];
if (c.scheduledForRemoval) { if (c.scheduledForRemoval) {
disconnect(c.connection, nullptr, this, nullptr); disconnect(c.connection, nullptr, this, nullptr);
c.connection->deleteLater(); c.connection->deleteLater();
@@ -232,44 +194,38 @@ private:
// We expect the number of concurrently open connections to be small. // We expect the number of concurrently open connections to be small.
// If that turns out to not be the case, we can still use a data // If that turns out to not be the case, we can still use a data
// structure with faster access. // structure with faster access.
QList<UnaquiredConnection> m_unacquiredConnections; QList<UnacquiredConnection> m_unacquiredConnections;
// Can contain the same connection more than once; this acts as a reference count. // Can contain the same connection more than once; this acts as a reference count.
QList<SshConnection *> m_acquiredConnections; QList<SshConnection *> m_acquiredConnections;
QList<SshConnection *> m_deprecatedConnections; QList<SshConnection *> m_deprecatedConnections;
QMutex m_listMutex;
QTimer m_removalTimer; QTimer m_removalTimer;
}; };
} // namespace Internal static SshConnectionManager *instance()
static QMutex instanceMutex;
static Internal::SshConnectionManager &instance()
{ {
static Internal::SshConnectionManager manager; QTC_CHECK(QThread::currentThread() == qApp->thread());
return manager; if (s_instance == nullptr)
s_instance = new SshConnectionManager(qApp); // Deleted together with application
return s_instance;
} }
} // namespace Internal
SshConnection *acquireConnection(const SshConnectionParameters &sshParams) SshConnection *acquireConnection(const SshConnectionParameters &sshParams)
{ {
QMutexLocker locker(&instanceMutex); return Internal::instance()->acquireConnection(sshParams);
return instance().acquireConnection(sshParams);
} }
void releaseConnection(SshConnection *connection) void releaseConnection(SshConnection *connection)
{ {
QMutexLocker locker(&instanceMutex); Internal::instance()->releaseConnection(connection);
instance().releaseConnection(connection);
} }
void forceNewConnection(const SshConnectionParameters &sshParams) void forceNewConnection(const SshConnectionParameters &sshParams)
{ {
QMutexLocker locker(&instanceMutex); Internal::instance()->forceNewConnection(sshParams);
instance().forceNewConnection(sshParams);
} }
} // namespace QSsh } // namespace QSsh
#include "sshconnectionmanager.moc"