Bug 1564: Stalled connection attempts can be canceled promptly

https://winscp.net/tracker/1564

Source commit: 47453d4644659b522e97fbba938e3588f24e7cb8

source/core/Queue.cpp

@@ -254,7 +254,7 @@ protected:
   bool FPause;
 
   virtual void __fastcall ProcessEvent();
-  virtual void __fastcall Finished();
+  virtual bool __fastcall Finished();
   bool __fastcall WaitForUserAction(TQueueItem::TStatus ItemStatus, TUserAction * UserAction);
   bool __fastcall OverrideItemStatus(TQueueItem::TStatus & ItemStatus);
 
@@ -288,7 +288,10 @@ int __fastcall TSimpleThread::ThreadProc(void * Thread)
     DebugFail();
   }
   SimpleThread->FFinished = true;
-  SimpleThread->Finished();
+  if (SimpleThread->Finished())
+  {
+    delete SimpleThread;
+  }
   return 0;
 }
 //---------------------------------------------------------------------------
@@ -322,8 +325,9 @@ void __fastcall TSimpleThread::Start()
   }
 }
 //---------------------------------------------------------------------------
-void __fastcall TSimpleThread::Finished()
+bool __fastcall TSimpleThread::Finished()
 {
+  return false;
 }
 //---------------------------------------------------------------------------
 void __fastcall TSimpleThread::Close()
@@ -1428,11 +1432,13 @@ bool __fastcall TTerminalItem::WaitForUserAction(
   return Result;
 }
 //---------------------------------------------------------------------------
-void __fastcall TTerminalItem::Finished()
+bool __fastcall TTerminalItem::Finished()
 {
-  TSignalThread::Finished();
+  bool Result = TSignalThread::Finished();
 
   FQueue->TerminalFinished(this);
+
+  return Result;
 }
 //---------------------------------------------------------------------------
 void __fastcall TTerminalItem::TerminalQueryUser(TObject * Sender,
@@ -2323,6 +2329,8 @@ __fastcall TTerminalThread::TTerminalThread(TTerminal * Terminal) :
   FOnIdle = NULL;
   FUserAction = NULL;
   FCancel = false;
+  FAbandoned = false;
+  FAllowAbandon = false;
   FMainThread = GetCurrentThreadId();
   FSection = new TCriticalSection();
 
@@ -2380,11 +2388,16 @@ __fastcall TTerminalThread::~TTerminalThread()
   FTerminal->OnInitializeLog = FOnInitializeLog;
 
   delete FSection;
+  if (FAbandoned)
+  {
+    delete FTerminal;
+  }
 }
 //---------------------------------------------------------------------------
 void __fastcall TTerminalThread::Cancel()
 {
   FCancel = true;
+  FCancelAfter = IncMilliSecond(Now(), 1000);
 }
 //---------------------------------------------------------------------------
 void __fastcall TTerminalThread::Idle()
@@ -2467,11 +2480,25 @@ void __fastcall TTerminalThread::RunAction(TNotifyEvent Action)
           default:
             throw Exception(L"Error waiting for background session task to complete");
         }
+
+        if (AllowAbandon && !Done && FCancel && (Now() >= FCancelAfter))
+        {
+          TGuard Guard(FSection);
+          if (WaitForSingleObject(FActionEvent, 0) != WAIT_OBJECT_0)
+          {
+            FAbandoned = true;
+            FCancelled = true;
+            FatalAbort();
+          }
+        }
       }
       while (!Done);
 
 
-      Rethrow(FException);
+      if (Done)
+      {
+        Rethrow(FException);
+      }
     }
     __finally
     {
@@ -2519,7 +2546,13 @@ void __fastcall TTerminalThread::ProcessEvent()
     SaveException(E, FException);
   }
 
-  SetEvent(FActionEvent);
+  {
+    TGuard Guard(FSection);
+    if (!FAbandoned)
+    {
+      SetEvent(FActionEvent);
+    }
+  }
 }
 //---------------------------------------------------------------------------
 void __fastcall TTerminalThread::Rethrow(Exception *& Exception)
@@ -2577,7 +2610,7 @@ void __fastcall TTerminalThread::WaitForUserAction(TUserAction * UserAction)
     DebugAssert(Thread == FThreadId);
 
     bool DoCheckCancel =
-      DebugAlwaysFalse(UserAction == NULL) || !UserAction->Force();
+      DebugAlwaysFalse(UserAction == NULL) || !UserAction->Force() || FAbandoned;
     if (DoCheckCancel)
     {
       CheckCancel();
@@ -2799,3 +2832,22 @@ void __fastcall TTerminalThread::TerminalReadDirectoryProgress(
   Cancel = Action.Cancel;
 }
 //---------------------------------------------------------------------------
+bool __fastcall TTerminalThread::Release()
+{
+  bool Result = !FAbandoned;
+  if (Result)
+  {
+    delete this;
+  }
+  else
+  {
+    // only now has the owner released ownership of the thread, so we are safe to kill outselves.
+    Terminate();
+  }
+  return Result;
+}
+//---------------------------------------------------------------------------
+bool __fastcall TTerminalThread::Finished()
+{
+  return TSimpleThread::Finished() || FAbandoned;
+}

source/core/Queue.h

@@ -23,7 +23,7 @@ protected:
   bool FFinished;
 
   virtual void __fastcall Execute() = 0;
-  virtual void __fastcall Finished();
+  virtual bool __fastcall Finished();
 
   static int __fastcall ThreadProc(void * Thread);
 };
@@ -370,13 +370,17 @@ public:
   void __fastcall TerminalReopen();
 
   void __fastcall Cancel();
+  bool __fastcall Release();
   void __fastcall Idle();
 
   __property TNotifyEvent OnIdle = { read = FOnIdle, write = FOnIdle };
   __property bool Cancelling = { read = FCancel };
+  __property bool AllowAbandon = { read = FAllowAbandon, write = FAllowAbandon };
+
 
 protected:
   virtual void __fastcall ProcessEvent();
+  virtual bool __fastcall Finished();
 
 private:
   TTerminal * FTerminal;
@@ -401,8 +405,11 @@ private:
   Exception * FException;
   Exception * FIdleException;
   bool FCancel;
+  TDateTime FCancelAfter;
+  bool FAbandoned;
   bool FCancelled;
   bool FPendingIdle;
+  bool FAllowAbandon;
 
   DWORD FMainThread;
   TCriticalSection * FSection;

source/windows/TerminalManager.cpp

@@ -233,6 +233,7 @@ void __fastcall TTerminalManager::DoConnectTerminal(TTerminal * Terminal, bool R
   try
   {
     TTerminalThread * TerminalThread = new TTerminalThread(Terminal);
+    TerminalThread->AllowAbandon = (Terminal == FActiveTerminal);
     try
     {
       if (ManagedTerminal != NULL)
@@ -262,12 +263,31 @@ void __fastcall TTerminalManager::DoConnectTerminal(TTerminal * Terminal, bool R
     }
     __finally
     {
-      if (ManagedTerminal != NULL)
+      TerminalThread->OnIdle = NULL;
+      if (!TerminalThread->Release() && (DebugAlwaysTrue(Terminal == FActiveTerminal)))
       {
-        ManagedTerminal->TerminalThread = NULL;
+        // terminal was abandoned, must create a new one to replace it
+        Terminal = CreateTerminal(new TSessionData(L""));
+        SetupTerminal(Terminal);
+        OwnsObjects = false;
+        Items[ActiveTerminalIndex] = Terminal;
+        OwnsObjects = true;
+        FActiveTerminal = Terminal;
+
+        // when abandoning cancelled terminal, the form remains open
+        if (FAuthenticateForm != NULL)
+        {
+          delete FAuthenticateForm;
+          FAuthenticateForm = NULL;
+        }
+      }
+      else
+      {
+        if (ManagedTerminal != NULL)
+        {
+          ManagedTerminal->TerminalThread = NULL;
+        }
       }
-
-      delete TerminalThread;
     }
   }
   __finally
@@ -283,6 +303,8 @@ void __fastcall TTerminalManager::DoConnectTerminal(TTerminal * Terminal, bool R
 bool __fastcall TTerminalManager::ConnectTerminal(TTerminal * Terminal)
 {
   bool Result = true;
+  // were it an active terminal, it would allow abandoning, what this API cannot deal with
+  DebugAssert(Terminal != FActiveTerminal);
   try
   {
     DoConnectTerminal(Terminal, false);