Repository: microsoft/terminal (language: cpp)

Issue:
[DefTerm] Some invocations can cause deadlocks in terminal + another GUI application in Attach mode
The following conditions must be true:

1. Terminal is set as the default
2. Terminal is set to "Attach" new instances "to the most recently used window"
3. Terminal is running

If so, the following application will deadlock:

```c++
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#include <stdlib.h>
#include <string_view>

int APIENTRY wWinMain(HINSTANCE hInstance, HINSTANCE, LPWSTR, int)
{
	static constexpr auto szWindowClass{ L"BlahWindowClass" };
	static constexpr std::wstring_view szMessage{ L"Launch Windows Terminal w/ \"Attach to most recent...\" set\nClick anywhere to deadlock" };

	auto WndProc = [](HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) -> LRESULT {
		switch (message)
		{
		case WM_LBUTTONUP:
			system("cmd /s /c echo hi");
			break;
		case WM_PAINT:
		{
			PAINTSTRUCT ps;
			HDC hdc = BeginPaint(hWnd, &ps);
			RECT r{ 0, 0, 500, 200 };
			DrawTextW(hdc, szMessage.data(), szMessage.size(), &r, DT_LEFT);
			EndPaint(hWnd, &ps);
		}
		break;
		case WM_DESTROY:
			PostQuitMessage(0);
			break;
		default:
			return DefWindowProcW(hWnd, message, wParam, lParam);
		}
		return 0;
	};

	WNDCLASSEXW wcex{};
	wcex.cbSize = sizeof(WNDCLASSEX);
	wcex.lpfnWndProc = static_cast<LRESULT(CALLBACK*)(HWND, UINT, WPARAM, LPARAM)>(WndProc);
	wcex.hInstance = hInstance;
	wcex.hCursor = LoadCursorW(nullptr, (LPCWSTR)IDC_ARROW);
	wcex.hbrBackground = (HBRUSH)(COLOR_WINDOW + 1);
	wcex.lpszClassName = szWindowClass;
	RegisterClassExW(&wcex);

	HWND hWnd = CreateWindowExW(0, szWindowClass, L"Title", WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, 0, 500, 200, nullptr, nullptr, hInstance, nullptr);

	ShowWindow(hWnd, SW_SHOWDEFAULT);
	UpdateWindow(hWnd);

	MSG msg;
	while (GetMessage(&msg, nullptr, 0, 0))
	{
		TranslateMessage(&msg);
		DispatchMessage(&msg);
	}

	return (int)msg.wParam;
}
```

Save it as `foo.cpp` and compile:

```
cl /std:c++17 foo.cpp /link onecoreuap.lib
```

Launch `foo.exe` and follow the instruction.

---

Windows Terminal attempts to bring itself to the foreground by calling `AttachThreadInput`, `BringToForeground` and `AttachThreadInput` in rapid succession.
When the application that is currently in the foreground is calling out to `system()` to spawn a commandline application, `BringToForeground` will fail: nobody
is running a message loop, and therefore no win32 messages are being drained. That application is waiting for `system()` to return before processing further
messages.

This may seem like a theoretical problem, but I encountered it while trying to play [The Battle for Wesnoth](https://www.wesnoth.org/) while on vacation :grinning:



Discussion / hints:

Fix patch:
diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp
index 83b572a082c..cb3425c1eff 100644
--- a/src/cascadia/WindowsTerminal/IslandWindow.cpp
+++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp
@@ -1508,23 +1508,31 @@ void IslandWindow::_globalActivateWindow(const uint32_t dropdownDuration,
     }
     else
     {
-        const auto windowThreadProcessId = GetWindowThreadProcessId(oldForegroundWindow, nullptr);
-        const auto currentThreadId = GetCurrentThreadId();
-
-        LOG_IF_WIN32_BOOL_FALSE(AttachThreadInput(windowThreadProcessId, currentThreadId, true));
-        // Just in case, add the thread detach as a scope_exit, to make _sure_ we do it.
-        auto detachThread = wil::scope_exit([windowThreadProcessId, currentThreadId]() {
-            LOG_IF_WIN32_BOOL_FALSE(AttachThreadInput(windowThreadProcessId, currentThreadId, false));
-        });
-        LOG_IF_WIN32_BOOL_FALSE(BringWindowToTop(_window.get()));
-        ShowWindow(_window.get(), SW_SHOW);
-
-        // Activate the window too. This will force us to the virtual desktop this
-        // window is on, if it's on another virtual desktop.
-        LOG_LAST_ERROR_IF_NULL(SetActiveWindow(_window.get()));
-
-        // Throw us on the active monitor.
-        _moveToMonitor(oldForegroundWindow, toMonitor);
+        // Try first to send a message to the current foreground window. If it's not responding, it may
+        // be waiting on us to finsh launching. Passing SMTO_NOTIMEOUTIFNOTHUNG means that we get the same
+        // behavior as before--that is, waiting for the message loop--but we've done an early return if
+        // it turns out that it was hung.
+        // SendMessageTimeoutW returns nonzero if it succeeds.
+        if (0 != SendMessageTimeoutW(oldForegroundWindow, WM_NULL, 0, 0, SMTO_NOTIMEOUTIFNOTHUNG | SMTO_BLOCK | SMTO_ABORTIFHUNG, 1000, nullptr))
+        {
+            const auto windowThreadProcessId = GetWindowThreadProcessId(oldForegroundWindow, nullptr);
+            const auto currentThreadId = GetCurrentThreadId();
+
+            LOG_IF_WIN32_BOOL_FALSE(AttachThreadInput(windowThreadProcessId, currentThreadId, true));
+            // Just in case, add the thread detach as a scope_exit, to make _sure_ we do it.
+            auto detachThread = wil::scope_exit([windowThreadProcessId, currentThreadId]() {
+                LOG_IF_WIN32_BOOL_FALSE(AttachThreadInput(windowThreadProcessId, currentThreadId, false));
+            });
+            LOG_IF_WIN32_BOOL_FALSE(BringWindowToTop(_window.get()));
+            ShowWindow(_window.get(), SW_SHOW);
+
+            // Activate the window too. This will force us to the virtual desktop this
+            // window is on, if it's on another virtual desktop.
+            LOG_LAST_ERROR_IF_NULL(SetActiveWindow(_window.get()));
+
+            // Throw us on the active monitor.
+            _moveToMonitor(oldForegroundWindow, toMonitor);
+        }
     }
 }
 


Test patch: