[cont] [DefTerm] Some invocations can cause deadlocks in terminal + another GUI application in Attach mode

[FuPeiJiang]

Windows Terminal version

No response

Windows build number

No response

Other Software

No response

Steps to reproduce

Same steps as #14131
More here: #14131 (comment)

Expected Behavior

no 5000ms timeout from SendMessageTimeoutW: SMTO_NOTIMEOUTIFNOTHUNG

Actual Behavior

5000ms because the window is newly hung (by call to system())

[DHowett]

I've got some prototype work to fix this over in 6e63704. I authored it some time in 2024, then lost track of it. I'll tag this up for the backlog. Thanks for the new notes.

[FuPeiJiang]

if I understand correctly, a new process (shim.cpp) sets AllowSetForegroundWindow
(since new processes can call AllowSetForegroundWindow or SetForegroundWindow)

during testing, this works perfectly for "Create a new window" (New instance behavior)
but it doesn't work reliably for "Attach to the most recently used window"

highlight/flashing occurs when SetForegroundWindow fails

I think a new process needs to be spawned to call SetForegroundWindow for it,
since it's not allowed to call SetForegroundWindow anymore (AllowSetForegroundWindow expired)
you can always check if it's still allowed to by calling AllowSetForegroundWindow on itself

---

when I switch back to "Create a new window", and run system many times (by clicking), I eventually run into a crash:

    winrt::Windows::Foundation::Size TermControl::MinimumSize()
    {
        //...
        else
        {
            // Do we ever get here (= uninitialized terminal)? If so: How?
            assert(false);
            return { 10, 10 };
        }

windbg:

0:000> k
 # Child-SP          RetAddr               Call Site
00 ucrtbased!issue_debug_notification+0x45 [minkernel\crts\ucrt\src\appcrt\internal\report_runtime_error.cpp @ 28]
01 ucrtbased!__acrt_report_runtime_error+0x13 [minkernel\crts\ucrt\src\appcrt\internal\report_runtime_error.cpp @ 154]
02 ucrtbased!abort+0x1d [minkernel\crts\ucrt\src\appcrt\startup\abort.cpp @ 61]
03 ucrtbased!common_assert_to_message_box<wchar_t>+0xdf [minkernel\crts\ucrt\src\appcrt\startup\assert.cpp @ 400]
04 ucrtbased!common_assert<wchar_t>+0x83 [minkernel\crts\ucrt\src\appcrt\startup\assert.cpp @ 424]
05 ucrtbased!_wassert+0x2f [minkernel\crts\ucrt\src\appcrt\startup\assert.cpp @ 444]
06 Microsoft_Terminal_Control!winrt::Microsoft::Terminal::Control::implementation::TermControl::MinimumSize+0x25f [terminal\src\cascadia\TerminalControl\TermControl.cpp @ 2869]
07 Microsoft_Terminal_Control!winrt::impl::produce<winrt::Microsoft::Terminal::Control::implementation::TermControl,winrt::Microsoft::Terminal::Control::ITermControl>::get_MinimumSize+0x101 [terminal\src\cascadia\TerminalControl\Generated Files\winrt\Microsoft.Terminal.Control.h @ 10170]
08 TerminalApp!winrt::impl::consume_Microsoft_Terminal_Control_ITermControl<winrt::Microsoft::Terminal::Control::ITermControl>::MinimumSize+0xeb [terminal\src\cascadia\TerminalApp\Generated Files\winrt\Microsoft.Terminal.Control.h @ 6498]
09 TerminalApp!winrt::TerminalApp::implementation::TerminalPaneContent::MinimumSize+0x3a [terminal\src\cascadia\TerminalApp\TerminalPaneContent.cpp @ 61]
0a TerminalApp!winrt::impl::produce<winrt::TerminalApp::implementation::TerminalPaneContent,winrt::TerminalApp::IPaneContent>::get_MinimumSize+0x101 [terminal\src\cascadia\TerminalApp\Generated Files\winrt\TerminalApp.h @ 12329]
0b TerminalApp!winrt::impl::consume_TerminalApp_IPaneContent<winrt::TerminalApp::IPaneContent>::MinimumSize+0xe8 [terminal\src\cascadia\TerminalApp\Generated Files\winrt\TerminalApp.h @ 2226]
0c TerminalApp!Pane::_GetMinSize+0x8a [terminal\src\cascadia\TerminalApp\Pane.cpp @ 2859]
0d TerminalApp!Pane::_CalcSnappedDimension+0x17b [terminal\src\cascadia\TerminalApp\Pane.cpp @ 2648]
0e TerminalApp!Pane::CalcSnappedDimension+0x63 [terminal\src\cascadia\TerminalApp\Pane.cpp @ 2619]
0f TerminalApp!winrt::TerminalApp::implementation::Tab::CalcSnappedDimension+0x17d [terminal\src\cascadia\TerminalApp\Tab.cpp @ 839]
10 TerminalApp!winrt::TerminalApp::implementation::TerminalPage::CalcSnappedDimension+0x1c3 [terminal\src\cascadia\TerminalApp\TerminalPage.cpp @ 2928]
11 TerminalApp!winrt::TerminalApp::implementation::TerminalWindow::CalcSnappedDimension+0x50 [terminal\src\cascadia\TerminalApp\TerminalWindow.cpp @ 782]
12 TerminalApp!winrt::impl::produce<winrt::TerminalApp::implementation::TerminalWindow,winrt::TerminalApp::ITerminalWindow>::CalcSnappedDimension+0x10e [terminal\src\cascadia\TerminalApp\Generated Files\winrt\TerminalApp.h @ 15261]
13 WindowsTerminal!winrt::impl::consume_TerminalApp_ITerminalWindow<winrt::TerminalApp::ITerminalWindow>::CalcSnappedDimension+0xe7 [terminal\src\cascadia\WindowsTerminal\Generated Files\winrt\TerminalApp.h @ 9562]
14 WindowsTerminal!std::_Func_impl_no_alloc<`AppHost::Initialize'::`2'::<lambda_5>,float,bool,float>::_Do_call+0x4e [C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.44.35207\include\functional @ 882]
15 WindowsTerminal!std::_Func_class<float,bool,float>::operator()+0xd1 [C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.44.35207\include\functional @ 927]
16 WindowsTerminal!IslandWindow::_OnGetMinMaxInfo+0x181 [terminal\src\cascadia\WindowsTerminal\IslandWindow.cpp @ 469]
17 WindowsTerminal!IslandWindow::MessageHandler+0x180 [terminal\src\cascadia\WindowsTerminal\IslandWindow.cpp @ 489]
18 WindowsTerminal!NonClientIslandWindow::MessageHandler+0x29a [terminal\src\cascadia\WindowsTerminal\NonClientIslandWindow.cpp @ 991]
19 WindowsTerminal!BaseWindow<IslandWindow>::WndProc+0x2bf [terminal\src\cascadia\WindowsTerminal\BaseWindow.h @ 33]
1a user32!UserCallWinProcCheckWow+0x2d1
1b user32!DispatchClientMessage+0x9c
1c user32!_fnINOUTLPPOINT5+0x3c
1d ntdll!KiUserCallbackDispatcherContinue
1e win32u!NtUserGetMessage+0x14
1f user32!GetMessageW+0x2a
20 WindowsTerminal!WindowEmperor::HandleCommandlineArgs+0xf5e [terminal\src\cascadia\WindowsTerminal\WindowEmperor.cpp @ 454]
21 WindowsTerminal!wWinMain+0x242 [terminal\src\cascadia\WindowsTerminal\main.cpp @ 125]
22 WindowsTerminal!invoke_main+0x32 [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl @ 123]
23 WindowsTerminal!__scrt_common_main_seh+0x132 [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl @ 288]
24 WindowsTerminal!__scrt_common_main+0xe [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl @ 331]
25 WindowsTerminal!wWinMainCRTStartup+0xe [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_wwinmain.cpp @ 17]
26 KERNEL32!BaseThreadInitThunk+0x1d
27 ntdll!RtlUserThreadStart+0x28

[DHowett]

if I understand correctly, a new process (shim.cpp) sets AllowSetForegroundWindow

Not exactly. Shim is only used when Terminal is launched via the wt alias. Shim is the wt alias.

The problem is not that SetForegroundWindow expires; rather, it is that the inbox console host doesn't propagate foreground rights to the COM server OpenConsole.exe, which cannot propagate foreground rights to WindowsTerminal.exe.

Even with comprehensive foreground forwarding from 7118d87, we still need some in-Windows changes to pass foreground rights from the originating console host.

[DHowett]

(It looks like the originating console host doesn't technically have foreground rights either, but rather, it typically gets by because it sets its window to be effectively owned by the application which spawned it. There's a lot more work here to do than it would appear.)

[FuPeiJiang]

conhost.exe -> OpenConsole.exe -> WindowsTerminal.exe

but rather, it typically gets by because it sets its window to be effectively owned by the application which spawned it.

This is weird, because being owned will result in the same bugs as AttachThreadInput (synchronous), but there is no deadlock with conhost.exe

Bonus reminder: If two windows are related by a parent/child relationship or owner/owned relationship, then their input queues are automatically attached. For example, if you do a Set­Parent where the parent and child are in different threads, you have just synchronized the two threads. This sort of cross-thread relationship is technically legal, but it is very difficult to manage correctly, so it should be avoided unless you really know what you’re doing. And if you are doing cross-thread or cross-process between windows that were not designed to participate in cross-thread or cross-process parenting, you are almost certainly doomed.

https://devblogs.microsoft.com/oldnewthing/20130607-00/?p=4143

what if the application which spawned conhost.exe doesn't have a window ? or doesn't have foreground rights ? then forwarding becomes impossible ?

could you confirm if it's possible for a new process to not have foreground rights ? Am I misunderstanding this ?

AllowSetForegroundWindow.c

#include <stdio.h>
#include <windows.h>

#pragma comment(lib, "user32.lib")

int main() {
    printf("%d\n", AllowSetForegroundWindow(ASFW_ANY));
    return 0;
}

I callCreateProcess AllowSetForegroundWindow.exe with CREATE_NO_WINDOW, but it can always call AllowSetForegroundWindow (successfully with return value 1)

I'm using AutoHotkey to test this because it supports hotkeys
(how else will I get a window that's not already foreground to create a new process ?)
RegisterHotKey -> WM_HOTKEY gives you foreground rights so that's a no go
but SetWindowsHookEx WH_KEYBOARD_LL -> PostMessage doesn't have foreground rights, which is what I'm using: $f1 vs f1

---

$f1::ToolTip DllCall("AllowSetForegroundWindow", "Uint", -1) " " StdoutToVar("AllowSetForegroundWindow.exe").Output
$f2::ToolTip DllCall("AllowSetForegroundWindow", "Uint", -1) ; returns 0

; ----------------------------------------------------------------------------------------------------------------------
; Function .....: StdoutToVar
; Description ..: Runs a command line program and returns its output.
; Parameters ...: sCmd - Commandline to be executed.
; ..............: sDir - Working directory.
; ..............: sEnc - Encoding used by the target process. Look at StrGet() for possible values.
; Return .......: Command output as a string on success, empty string on error.
; AHK Version ..: AHK v2 x32/64 Unicode
; Author .......: Sean (http://goo.gl/o3VCO8), modified by nfl and by Cyruz
; License ......: WTFPL - http://www.wtfpl.net/txt/copying/
; Changelog ....: Feb. 20, 2007 - Sean version.
; ..............: Sep. 21, 2011 - nfl version.
; ..............: Nov. 27, 2013 - Cyruz version (code refactored and exit code).
; ..............: Mar. 09, 2014 - Removed input, doesn't seem reliable. Some code improvements.
; ..............: Mar. 16, 2014 - Added encoding parameter as pointed out by lexikos.
; ..............: Jun. 02, 2014 - Corrected exit code error.
; ..............: Nov. 02, 2016 - Fixed blocking behavior due to ReadFile thanks to PeekNamedPipe.
; ..............: Apr. 13, 2021 - Code restyling. Fixed deprecated DllCall types.
; ..............: Oct. 06, 2022 - AHK v2 version. Throw exceptions on failure.
; ..............: Oct. 08, 2022 - Exceptions management and handles closure fix. Thanks to lexikos and iseahound.
; ----------------------------------------------------------------------------------------------------------------------
StdoutToVar(sCmd, sDir:="", sEnc:="CP0") {
    ; Create 2 buffer-like objects to wrap the handles to take advantage of the __Delete meta-function.
    oHndStdoutRd := { Ptr: 0, __Delete: delete(this) => DllCall("CloseHandle", "Ptr", this) }
    oHndStdoutWr := { Base: oHndStdoutRd }
    
    If !DllCall( "CreatePipe"
               , "PtrP" , oHndStdoutRd
               , "PtrP" , oHndStdoutWr
               , "Ptr"  , 0
               , "UInt" , 0 )
        Throw OSError(,, "Error creating pipe.")
    If !DllCall( "SetHandleInformation"
               , "Ptr"  , oHndStdoutWr
               , "UInt" , 1
               , "UInt" , 1 )
        Throw OSError(,, "Error setting handle information.")

    PI := Buffer(A_PtrSize == 4 ? 16 : 24,  0)
    SI := Buffer(A_PtrSize == 4 ? 68 : 104, 0)
    NumPut( "UInt", SI.Size,          SI,  0 )
    NumPut( "UInt", 0x100,            SI, A_PtrSize == 4 ? 44 : 60 )
    NumPut( "Ptr",  oHndStdoutWr.Ptr, SI, A_PtrSize == 4 ? 60 : 88 )
    NumPut( "Ptr",  oHndStdoutWr.Ptr, SI, A_PtrSize == 4 ? 64 : 96 )

    If !DllCall( "CreateProcess"
               , "Ptr"  , 0
               , "Str"  , sCmd
               , "Ptr"  , 0
               , "Ptr"  , 0
               , "Int"  , True
               , "UInt" , 0x08000000
               , "Ptr"  , 0
               , "Ptr"  , sDir ? StrPtr(sDir) : 0
               , "Ptr"  , SI
               , "Ptr"  , PI )
        Throw OSError(,, "Error creating process.")

    ; The write pipe must be closed before reading the stdout so we release the object.
    ; The reading pipe will be released automatically on function return.
    oHndStdOutWr := ""

    ; Before reading, we check if the pipe has been written to, so we avoid freezings.
    nAvail := 0, nLen := 0
    While DllCall( "PeekNamedPipe"
                 , "Ptr"   , oHndStdoutRd
                 , "Ptr"   , 0
                 , "UInt"  , 0
                 , "Ptr"   , 0
                 , "UIntP" , &nAvail
                 , "Ptr"   , 0 ) != 0
    {
        ; If the pipe buffer is empty, sleep and continue checking.
        If !nAvail && Sleep(100)
            Continue
        cBuf := Buffer(nAvail+1)
        DllCall( "ReadFile"
               , "Ptr"  , oHndStdoutRd
               , "Ptr"  , cBuf
               , "UInt" , nAvail
               , "PtrP" , &nLen
               , "Ptr"  , 0 )
        sOutput .= StrGet(cBuf, nLen, sEnc)
    }
    
    ; Get the exit code, close all process handles and return the output object.
    DllCall( "GetExitCodeProcess"
           , "Ptr"   , NumGet(PI, 0, "Ptr")
           , "UIntP" , &nExitCode:=0 )
    DllCall( "CloseHandle", "Ptr", NumGet(PI, 0, "Ptr") )
    DllCall( "CloseHandle", "Ptr", NumGet(PI, A_PtrSize, "Ptr") )
    Return { Output: sOutput, ExitCode: nExitCode } 
}