SSH: Delay channel close request to server in SessionRequested state.

If closeChannel() is called in between our channel open request to the
server and the corresponding reply, we cannot forward the close request
to the server, as we don't have its channel id yet. So wait until we do.

Our failure to correctly handle this sequence of events was the root cause
for the following user-visible errors:
    - A (since-removed) Q_ASSERT in ~SshRemoteProcess() was hit.
    - The server closed the connection because we referred to an invalid
      channel ("Received ieof for nonexistent channel -1").

Commits 26920307f0 and 3027bcc952 are also related to this issue.

Change-Id: I4994d85f5b21a72682f75389cdf8769738bd6768
Reviewed-by: hjk <hjk121@nokiamail.com>

2 files changed, 15 insertions, 6 deletions

diff --git a/src/libs/ssh/sshchannel.cpp b/src/libs/ssh/sshchannel.cpp
index 269cee765bc..6e060356b49 100644
--- a/src/libs/ssh/sshchannel.cpp
+++ b/src/libs/ssh/sshchannel.cpp
@@ -137,11 +137,11 @@ void AbstractSshChannel::flushSendBuffer()
 void AbstractSshChannel::handleOpenSuccess(quint32 remoteChannelId,
     quint32 remoteWindowSize, quint32 remoteMaxPacketSize)
 {
-    switch (m_state) {
+    const ChannelState oldState = m_state;
+    switch (oldState) {
+    case CloseRequested:   // closeChannel() was called while we were in SessionRequested state
     case SessionRequested:
         break; // Ok, continue.
-    case CloseRequested:
-        return; // Late server reply; we requested a channel close in the meantime.
     default:
         throw SSH_SERVER_EXCEPTION(SSH_DISCONNECT_PROTOCOL_ERROR,
             "Unexpected SSH_MSG_CHANNEL_OPEN_CONFIRMATION packet.");
@@ -163,7 +163,10 @@ void AbstractSshChannel::handleOpenSuccess(quint32 remoteChannelId,
    m_remoteWindowSize = remoteWindowSize;
    m_remoteMaxPacketSize = remoteMaxPacketSize;
    setChannelState(SessionEstablished);
-   handleOpenSuccessInternal();
+   if (oldState == CloseRequested)
+       closeChannel();
+   else
+       handleOpenSuccessInternal();
 }
 
 void AbstractSshChannel::handleOpenFailure(const QString &reason)
@@ -260,8 +263,12 @@ void AbstractSshChannel::closeChannel()
             setChannelState(Closed);
         } else {
             setChannelState(CloseRequested);
-            m_sendFacility.sendChannelEofPacket(m_remoteChannel);
-            m_sendFacility.sendChannelClosePacket(m_remoteChannel);
+            if (m_remoteChannel != NoChannel) {
+                m_sendFacility.sendChannelEofPacket(m_remoteChannel);
+                m_sendFacility.sendChannelClosePacket(m_remoteChannel);
+            } else {
+                QSSH_ASSERT(m_state == SessionRequested);
+            }
         }
     }
 }
diff --git a/src/libs/ssh/sshremoteprocess.cpp b/src/libs/ssh/sshremoteprocess.cpp
index bd8bd6dc9fb..37bc172e7cf 100644
--- a/src/libs/ssh/sshremoteprocess.cpp
+++ b/src/libs/ssh/sshremoteprocess.cpp
@@ -31,6 +31,7 @@
 #include "sshremoteprocess.h"
 #include "sshremoteprocess_p.h"
 
+#include "ssh_global.h"
 #include "sshincomingpacket_p.h"
 #include "sshsendfacility_p.h"
 
@@ -85,6 +86,7 @@ SshRemoteProcess::SshRemoteProcess(quint32 channelId, Internal::SshSendFacility
 
 SshRemoteProcess::~SshRemoteProcess()
 {
+    QSSH_ASSERT(d->channelState() != Internal::AbstractSshChannel::SessionEstablished);
     close();
     delete d;
 }