From c1a89d2c6187aaae5ce607e9e6ac5b57c5aa1f2f Mon Sep 17 00:00:00 2001
From: Christian Kandeler <christian.kandeler@nokia.com>
Date: Wed, 20 Jun 2012 15:55:30 +0200
Subject: [PATCH] SSH: Close channels before re-using a connection.

Otherwise a new client acquiring the connection could be affected by
things happening in channels that were not opened by that client, which
would certainly be unexpected.
In particular, if the new owner of the connection runs in a different
thread than the old one, crashes could occur since the connection
assumes its channels run in the same thread.

Change-Id: I4fdf2b5a3751ed506631d6878e94342da033c31c
Reviewed-by: Tobias Hunger <tobias.hunger@nokia.com>
---
 src/libs/ssh/sshchannelmanager.cpp    | 4 +++-
 src/libs/ssh/sshchannelmanager_p.h    | 2 +-
 src/libs/ssh/sshconnection.cpp        | 9 +++++++++
 src/libs/ssh/sshconnection.h          | 3 +++
 src/libs/ssh/sshconnectionmanager.cpp | 9 +++++++--
 5 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/libs/ssh/sshchannelmanager.cpp b/src/libs/ssh/sshchannelmanager.cpp
index 409f2a586f3..74fe368885f 100644
--- a/src/libs/ssh/sshchannelmanager.cpp
+++ b/src/libs/ssh/sshchannelmanager.cpp
@@ -178,12 +178,14 @@ void SshChannelManager::insertChannel(AbstractSshChannel *priv,
     m_sessions.insert(priv, pub);
 }
 
-void SshChannelManager::closeAllChannels()
+int SshChannelManager::closeAllChannels()
 {
+    const int count = m_channels.count();
     for (ChannelIterator it = m_channels.begin(); it != m_channels.end(); ++it)
         it.value()->closeChannel();
     m_channels.clear();
     m_sessions.clear();
+    return count;
 }
 
 void SshChannelManager::removeChannel(ChannelIterator it)
diff --git a/src/libs/ssh/sshchannelmanager_p.h b/src/libs/ssh/sshchannelmanager_p.h
index 9cc88e9dd68..6ef4a406a06 100644
--- a/src/libs/ssh/sshchannelmanager_p.h
+++ b/src/libs/ssh/sshchannelmanager_p.h
@@ -57,7 +57,7 @@ public:
     QSharedPointer<SshRemoteProcess> createRemoteProcess(const QByteArray &command);
     QSharedPointer<SshRemoteProcess> createRemoteShell();
     QSharedPointer<SftpChannel> createSftpChannel();
-    void closeAllChannels();
+    int closeAllChannels();
 
     void handleChannelRequest(const SshIncomingPacket &packet);
     void handleChannelOpen(const SshIncomingPacket &packet);
diff --git a/src/libs/ssh/sshconnection.cpp b/src/libs/ssh/sshconnection.cpp
index 21cc7f57538..0337563c80d 100644
--- a/src/libs/ssh/sshconnection.cpp
+++ b/src/libs/ssh/sshconnection.cpp
@@ -193,6 +193,15 @@ QSharedPointer<SftpChannel> SshConnection::createSftpChannel()
     return d->createSftpChannel();
 }
 
+int SshConnection::closeAllChannels()
+{
+    try {
+        return d->m_channelManager->closeAllChannels();
+    } catch (const Botan::Exception &e) {
+        qDebug("%s: %s", Q_FUNC_INFO, e.what());
+        return -1;
+    }
+}
 
 namespace Internal {
 
diff --git a/src/libs/ssh/sshconnection.h b/src/libs/ssh/sshconnection.h
index 5354ec59463..5f70c282f57 100644
--- a/src/libs/ssh/sshconnection.h
+++ b/src/libs/ssh/sshconnection.h
@@ -106,6 +106,9 @@ public:
     QSharedPointer<SshRemoteProcess> createRemoteShell();
     QSharedPointer<SftpChannel> createSftpChannel();
 
+    // -1 if an error occurred, number of channels closed otherwise.
+    int closeAllChannels();
+
 signals:
     void connected();
     void disconnected();
diff --git a/src/libs/ssh/sshconnectionmanager.cpp b/src/libs/ssh/sshconnectionmanager.cpp
index 114b5200867..dc62c9eeb7f 100644
--- a/src/libs/ssh/sshconnectionmanager.cpp
+++ b/src/libs/ssh/sshconnectionmanager.cpp
@@ -145,10 +145,15 @@ public:
                     break;
                 }
             }
-            if (!haveConnection)
+            if (!haveConnection) {
+                // Let's nag clients who release connections with open channels.
+                const int channelCount = connection->closeAllChannels();
+                QSSH_ASSERT(channelCount == 0);
+
                 m_unacquiredConnections.append(connection);
-            else
+            } else {
                 doDelete = true;
+            }
         }
 
         if (doDelete) {