Repository: microsoft/terminal (language: cpp)

Issue:
Terminal window "not responding"
### Windows Terminal version

1.18.2822.0

### Windows build number

10.0.19045.0

### Other Software

_No response_

### Steps to reproduce

I'm using a terminal window, and then not using it for a while and doing other stuff.

### Expected Behavior

_No response_

### Actual Behavior

The terminal window becomes nonresponsive. The content is gone. The window title shows "not responding" and I can only kill the process at that point.


Discussion / hints:
Does this happen consistently/? Or was this a on-off kinda thing? I'd guess that this is because we just pushed an update to the app on the Store, and the Store tried to kill the Terminal to install the update. 

If that's the case, the Terminal version should probably now be https://github.com/microsoft/terminal/releases/tag/v1.18.3181.0
This happened at least 10 times in the last few weeks, but I can't make it happen at will. My version right now is 1.18.2822.0.

Is there any way to find logs or something on my machine that would give us information about what's happening? 
That sounds a bit like a deadlock, so log files probably won't be of help. You could try to capture a dump of a hung process before killing it: https://github.com/microsoft/terminal/wiki/Troubleshooting-Tips#capturing-and-sending-dumps

That way we can see what threads are currently stuck and what the UI thread is most likely waiting on.
Thank you, I'll give it a try.

On Fri, Nov 17, 2023 at 3:11 PM Leonard Hecker ***@***.***>
wrote:

> That sounds a bit like a deadlock, so log files probably won't be of help.
> You could try to capture a dump of a hung process before killing it:
> https://github.com/microsoft/terminal/wiki/Troubleshooting-Tips#capturing-and-sending-dumps
>
> That way we can see what threads are currently stuck and what the UI
> thread is most likely waiting on.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/microsoft/terminal/issues/16332#issuecomment-1816398136>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAN3SV6PVZFZAWGCN4DXA3YE5PABAVCNFSM6AAAAAA7PWZ4CGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJWGM4TQMJTGY>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>

Today this happened again. I opened Process Explorer and saw these processes:

![image](https://github.com/microsoft/terminal/assets/56778/c68eaf35-89b5-4bb9-ba48-3cae62489300)

I wasn't sure which process you wanted a dump of, and whether it should be a mini-dump or a full dump, so I just made both mini- and full dumps of each of the 4 processes. They're available for download here: https://www.dropbox.com/scl/fo/ngzixgtvl9b4gms1ya0bl/h?rlkey=00vm814nojext99o8agdwwzu3&dl=0

Does that help you figure out why it's hanging? 
The only notable thing I can see is that the active window thread is stuck in a call to `DefWindowProc` with a `WM_INPUTLANGCHANGEREQUEST` message. Unfortunately, the dump ends at `NtUserMessageCall` which is just a syscall stub. Basically, the kernel-side stack trace is missing in the dump and I don't know how to fix that.

But assuming that it calls into the venerable `xxxRealDefWindowProc`, then the default `WM_INPUTLANGCHANGEREQUEST` handler tries to pass the request to the currently focused window using `SendMessage` which is synchronous. In other words, if the currently focused window is unresponsive then the currently active window will also deadlock. I'm not familiar with the ntuser implementation so I can't really tell if "currently focused window" is only in the current process or system-wide.

In case of the former the issue might be caused by the 2nd window thread in your dump because that one is frozen (aka: A previously closed window. Does the issue occur if you never had more than 1 window of the Terminal open at a time?). In case of the latter it might be caused by another application you have installed that is unresponsive. Also in case of the latter, it might be caused by `xxxActivateKeyboardLayout` which is called if the currently active window is already focused. I suspect that this call interacts with the "Text Input Management Service" via COM/RPC and that might also hang if it has a bug.
![i-know-some-of-these-words-mhmm](https://github.com/microsoft/terminal/assets/56778/ea5911b1-7431-4dad-b652-bf9a1a96dd90)

Thanks for the analysis. In that link were both mini-dumps and full dumps. Was the kernel-side stack trace missing also from the full dump?
Yep! I'm unfortunately not experienced enough with dumps to know why that is and a quick ~Google~ Bing didn't bring any answers to that either...
I mean, maybe a dumb question is: are you changing the input language / method / something like that? 

If you send /feedback via the Feedback Hub, we might be able to see if there's something else crashing on your machine that's correlated here. That's a real long shot though. 
@zadjii-msft 

> I mean, maybe a dumb question is: are you changing the input language / method / something like that?

I do change language often, between English and Hebrew. I usually don't do it in the shell because I don't use Hebrew in the shell. It's possible that this problem is caused by an input language change, but I'm not noticing that it's happening immediately after a language change. Right now I opened terminal and changed language a bunch of times, both in the terminal app and outside it. I also typed some Hebrew in the terminal for good measure. The terminal did not crash.

Therefore I can't really do the recording action suggested above. The problem is not reproducible, though it keeps happening again and again. 
We had a bit of a brain blast: Does this only occur after you've opened two simultaneous terminal windows, and closed one of them (but not both)? 

We suspect that this hang is due to the window refrigeration code in Terminal 1.18 that gets used on Windows 10. We're guessing the message gets sent to the closed, refrigerated window, which just doesn't pump it. 
> Does this only occur after you've opened two simultaneous terminal windows, and closed one of them (but not both)?

I tried that now, it didn't make the problem happen. But I often have multiple terminal windows open, and I routinely open new ones and close existing ones.

Fix patch:
diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt
index c3a8eb95f17..d56d87fd070 100644
--- a/.github/actions/spelling/expect/expect.txt
+++ b/.github/actions/spelling/expect/expect.txt
@@ -481,6 +481,7 @@ directio
 DIRECTX
 DISABLEDELAYEDEXPANSION
 DISABLENOSCROLL
+DISPATCHNOTIFY
 DISPLAYATTRIBUTE
 DISPLAYATTRIBUTEPROPERTY
 DISPLAYCHANGE
diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp
index 3168011a491..6a0e0ed698d 100644
--- a/src/cascadia/WindowsTerminal/AppHost.cpp
+++ b/src/cascadia/WindowsTerminal/AppHost.cpp
@@ -535,23 +535,19 @@ void AppHost::LastTabClosed(const winrt::Windows::Foundation::IInspectable& /*se
     {
         _windowLogic.ClearPersistedWindowState();
     }
-
-    // If the user closes the last tab, in the last window, _by closing the tab_
-    // (not by closing the whole window), we need to manually persist an empty
-    // window state here. That will cause the terminal to re-open with the usual
-    // settings (not the persisted state)
-    if (args.ClearPersistedState() &&
-        _windowManager.GetNumberOfPeasants() == 1)
-    {
-        _windowLogic.ClearPersistedWindowState();
-    }
-
     // Remove ourself from the list of peasants so that we aren't included in
     // any future requests. This will also mean we block until any existing
     // event handler finishes.
     _windowManager.SignalClose(_peasant);
 
-    PostQuitMessage(0);
+    if (Utils::IsWindows11())
+    {
+        PostQuitMessage(0);
+    }
+    else
+    {
+        PostMessageW(_window->GetInteropHandle(), WM_REFRIGERATE, 0, 0);
+    }
 }
 
 LaunchPosition AppHost::_GetWindowLaunchPosition()
diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h
index 992a4402874..60b96103124 100644
--- a/src/cascadia/WindowsTerminal/AppHost.h
+++ b/src/cascadia/WindowsTerminal/AppHost.h
@@ -11,6 +11,8 @@
 class AppHost : public std::enable_shared_from_this<AppHost>
 {
 public:
+    static constexpr DWORD WM_REFRIGERATE = WM_APP + 0;
+
     AppHost(const winrt::TerminalApp::AppLogic& logic,
             winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args,
             const winrt::Microsoft::Terminal::Remoting::WindowManager& manager,
diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp
index ee538bfa71a..bab65132266 100644
--- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp
+++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp
@@ -26,6 +26,11 @@ NonClientIslandWindow::NonClientIslandWindow(const ElementTheme& requestedTheme)
 {
 }
 
+NonClientIslandWindow::~NonClientIslandWindow()
+{
+    Close();
+}
+
 void NonClientIslandWindow::Close()
 {
     // Avoid further callbacks into XAML/WinUI-land after we've Close()d the DesktopWindowXamlSource
diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h
index d173ad68c2d..b2cbb5a8a29 100644
--- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h
+++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h
@@ -30,6 +30,7 @@ class NonClientIslandWindow : public IslandWindow
     static constexpr const int topBorderVisibleHeight = 1;
 
     NonClientIslandWindow(const winrt::Windows::UI::Xaml::ElementTheme& requestedTheme) noexcept;
+    ~NonClientIslandWindow() override;
 
     void Refrigerate() noexcept override;
 
diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp
index 47a11bba38e..ade38358593 100644
--- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp
+++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp
@@ -584,15 +584,6 @@ LRESULT WindowEmperor::_messageHandler(UINT const message, WPARAM const wParam,
 // we'll undoubtedly crash.
 winrt::fire_and_forget WindowEmperor::_close()
 {
-    {
-        auto fridge{ _oldThreads.lock() };
-        for (auto& window : *fridge)
-        {
-            window->ThrowAway();
-        }
-        fridge->clear();
-    }
-
     // Important! Switch back to the main thread for the emperor. That way, the
     // quit will go to the emperor's message pump.
     co_await wil::resume_foreground(_dispatcher);
diff --git a/src/cascadia/WindowsTerminal/WindowThread.cpp b/src/cascadia/WindowsTerminal/WindowThread.cpp
index 2c70741ad1c..bafe6ef97e5 100644
--- a/src/cascadia/WindowsTerminal/WindowThread.cpp
+++ b/src/cascadia/WindowsTerminal/WindowThread.cpp
@@ -4,6 +4,8 @@
 #include "pch.h"
 #include "WindowThread.h"
 
+using namespace winrt::Microsoft::Terminal::Remoting;
+
 WindowThread::WindowThread(winrt::TerminalApp::AppLogic logic,
                            winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args,
                            winrt::Microsoft::Terminal::Remoting::WindowManager manager,
@@ -81,17 +83,6 @@ void WindowThread::RundownForExit()
     _pumpRemainingXamlMessages();
 }
 
-void WindowThread::ThrowAway()
-{
-    // raise the signal to unblock KeepWarm. We won't have a host, so we'll drop
-    // out of the message loop to eventually RundownForExit.
-    //
-    // This should only be called when the app is fully quitting. After this is
-    // called on any thread, on win10, we won't be able to call into XAML
-    // anymore.
-    _microwaveBuzzer.notify_one();
-}
-
 // Method Description:
 // - Check if we should keep this window alive, to try it's message loop again.
 //   If we were refrigerated for later, then this will block the thread on the
@@ -108,29 +99,24 @@ bool WindowThread::KeepWarm()
         return true;
     }
 
-    // If we're refrigerated, then wait on the microwave signal, which will be
-    // raised when we get re-heated by another thread to reactivate us.
-
-    if (_warmWindow != nullptr)
+    // Even when the _host has been destroyed the HWND will continue receiving messages, in particular WM_DISPATCHNOTIFY at least once a second. This is important to Windows as it keeps your room warm.
+    MSG msg;
+    for (;;)
     {
-        std::unique_lock lock(_microwave);
-        _microwaveBuzzer.wait(lock);
-
-        // If ThrowAway() was called, then the buzzer will be signalled without
-        // setting a new _host. In that case, the app is quitting, for real. We
-        // just want to exit with false.
-        const bool reheated = _host != nullptr;
-        if (reheated)
+        if (!GetMessageW(&msg, nullptr, 0, 0))
+        {
+            return false;
+        }
+        // We're using a single window message (WM_REFRIGERATE) to indicate both
+        // state transitions. In this case, the window is actually being woken up.
+        if (msg.message == AppHost::WM_REFRIGERATE)
         {
             _UpdateSettingsRequestedToken = _host->UpdateSettingsRequested([this]() { _UpdateSettingsRequestedHandlers(); });
             // Re-initialize the host here, on the window thread
             _host->Initialize();
+            return true;
         }
-        return reheated;
-    }
-    else
-    {
-        return false;
+        DispatchMessageW(&msg);
     }
 }
 
@@ -154,24 +140,22 @@ void WindowThread::Refrigerate()
 
 // Method Description:
 // - "Reheat" this thread for reuse. We'll build a new AppHost, and pass in the
-//   existing window to it. We'll then trigger the _microwaveBuzzer, so KeepWarm
-//   (which is on the UI thread) will get unblocked, and we can initialize this
-//   window.
-void WindowThread::Microwave(
-    winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args,
-    winrt::Microsoft::Terminal::Remoting::Peasant peasant)
+//   existing window to it. We'll then wake up the thread stuck in KeepWarm().
+void WindowThread::Microwave(WindowRequestedArgs args, Peasant peasant)
 {
+    const auto hwnd = _warmWindow->GetInteropHandle();
+
     _peasant = std::move(peasant);
     _args = std::move(args);
 
-    _host = std::make_shared<::AppHost>(_appLogic,
-                                        _args,
-                                        _manager,
-                                        _peasant,
-                                        std::move(_warmWindow));
+    _host = std::make_shared<AppHost>(_appLogic,
+                                      _args,
+                                      _manager,
+                                      _peasant,
+                                      std::move(_warmWindow));
 
     // raise the signal to unblock KeepWarm and start the window message loop again.
-    _microwaveBuzzer.notify_one();
+    PostMessageW(hwnd, AppHost::WM_REFRIGERATE, 0, 0);
 }
 
 winrt::TerminalApp::TerminalWindow WindowThread::Logic()
@@ -198,6 +182,15 @@ int WindowThread::_messagePump()
 
     while (GetMessageW(&message, nullptr, 0, 0))
     {
+        // We're using a single window message (WM_REFRIGERATE) to indicate both
+        // state transitions. In this case, the window is actually being refrigerated.
+        // This will break us out of our main message loop we'll eventually start
+        // the loop in WindowThread::KeepWarm to await a call to Microwave().
+        if (message.message == AppHost::WM_REFRIGERATE)
+        {
+            break;
+        }
+
         // GH#638 (Pressing F7 brings up both the history AND a caret browsing message)
         // The Xaml input stack doesn't allow an application to suppress the "caret browsing"
         // dialog experience triggered when you press F7. Official recommendation from the Xaml
diff --git a/src/cascadia/WindowsTerminal/WindowThread.h b/src/cascadia/WindowsTerminal/WindowThread.h
index a1af2db6500..c536e4297aa 100644
--- a/src/cascadia/WindowsTerminal/WindowThread.h
+++ b/src/cascadia/WindowsTerminal/WindowThread.h
@@ -22,7 +22,6 @@ class WindowThread : public std::enable_shared_from_this<WindowThread>
     void Microwave(
         winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args,
         winrt::Microsoft::Terminal::Remoting::Peasant peasant);
-    void ThrowAway();
 
     uint64_t PeasantID();
 
@@ -43,8 +42,6 @@ class WindowThread : public std::enable_shared_from_this<WindowThread>
     winrt::event_token _UpdateSettingsRequestedToken;
 
     std::unique_ptr<::IslandWindow> _warmWindow{ nullptr };
-    std::mutex _microwave;
-    std::condition_variable _microwaveBuzzer;
 
     int _messagePump();
     void _pumpRemainingXamlMessages();


Test patch: