Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

conhost headless mode is broken #13914

Closed
AveYo opened this issue Sep 2, 2022 · 11 comments · Fixed by #13950
Closed

conhost headless mode is broken #13914

AveYo opened this issue Sep 2, 2022 · 11 comments · Fixed by #13950
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@AveYo
Copy link

AveYo commented Sep 2, 2022

Windows Terminal version

1.15.2203.0

Windows build number

10.0.25193.1000

Other Software

No response

Steps to reproduce

conhost --headless cmd /c "echo;%date% %time%>>%userprofile%\desktop\test.log"

Expected Behavior

test.log with date and time on desktop sans brief ui, like it worked so far up to build 25179

Actual Behavior

cmd.exe - Application Error dialog
The application was unable to start correctly (0xc0000142).
Click OK to close the application.

@AveYo AveYo added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Sep 2, 2022
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 2, 2022
@AveYo
Copy link
Author

AveYo commented Sep 2, 2022

Briefly checked commit history and #13832 or #13570 stand out.

@DHowett
Copy link
Member

DHowett commented Sep 2, 2022

Thanks for the report!

The direct invocation of conhost with its (admittedly undocumented) arguments isn't exactly "officially" supported.
conhost --headless primarily exists to support the CreatePseudoConsole API and its coconspirators. We've got some automated tests for those APIs and they haven't flagged anything as being amiss.

Despite that, I'm not inclined to close this issue because it probably represents an under-served need... and because we didn't intentionally break it.

What was the use case for which you were using --headless? I'd love for us to brainstorm ways to do whatever it is that we can fully support and can guarantee will continue to work or back up with APIs.

In general: spawning headless console applications is achievable via START /MIN or CreateProcess with DETACH... (sorry, can't remember the name today.)

@DHowett DHowett added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 2, 2022
@AveYo
Copy link
Author

AveYo commented Sep 3, 2022

Use case:

shortcuts launching various powershell-based GUI front-ends in plain-text scripts,

sans annoying / ocd-inducing flashing console window
I like to think of these as Progressive Script Apps - very mouse & touch friendly, and can't get more "open-source" than that, trivial to audit vs. compiled programs

command-line scripting of various brief and simple tasks,

sans annoying / ocd-inducing flashing console window
sans 3rd party tools fauna representing this under-served need these past.. decades
sans annoying / ocd-inducing false-positive harassment if using higher-scripting languages stubs via wscript, mshta & co. (introducing complexity to otherwise a simple need)
I extensively use it for admin / monitoring / intercepting programs via Image File Execution Options, and under user session the amount of console flashing can be quite overwhelming

parity with terminals on *nix multiple ways of running headless scripts, by design

Start /min was never a solution since it requires non-headless cmd - same for Run Minimized for shortcuts.
When the system is busy console flashes regardless, plus it makes going back to foreground even more difficult in the focus-deficient 11 taskbar (like, even UAC prompts don't pop-up often).
And even that has regressed with 11: sometimes window flashes, other times taskbar icon appears briefly, or not appear at all; it's worse under virtualization / modest hardware / busy system; uninstalling Terminal gives a more consistent taskbar icon appearance (and speaking of that, Win + X is broken again, does not revert to Powershell).

Compiled programs obviously have access to window creation parameters, but that's outside the scope of Command Prompt / Powershell / Terminal and scripts.

So maybe restore OS conhost behavior and do what's needed with Terminal's own OpenConsole?
Or outright deprecate this undocumented behavior across all windows versions and silently ignore headless mode.
Crashing cmd / powershell is definitely not nice.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Sep 3, 2022
@zadjii-msft zadjii-msft added the Needs-Discussion Something that requires a team discussion before we can proceed label Sep 6, 2022
@zadjii-msft
Copy link
Member

zadjii-msft commented Sep 6, 2022

Note to self: bin\x64\Debug\OpenConsole.exe --headless cmd /c "echo;%date% %time%>>%userprofile%\desktop\test.log"

0:002> k
 # Child-SP          RetAddr               Call Site
00 00000083`b78ffd10 00007ff6`f128b097     OpenConsole!std::optional<Microsoft::Console::PtySignalInputThread::SetParentData>::has_value+0x1b [C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31517\include\optional @ 416] 
01 00000083`b78ffd40 00007ff6`f1294f71     OpenConsole!Microsoft::Console::PtySignalInputThread::CreatePseudoWindow+0x27 [D:\dev\public\terminal\src\host\PtySignalInputThread.cpp @ 90] 
02 00000083`b78ffd80 00007ff6`f116a854     OpenConsole!Microsoft::Console::VirtualTerminal::VtIo::CreatePseudoWindow+0x31 [D:\dev\public\terminal\src\host\VtIo.cpp @ 329] 
03 00000083`b78ffdb0 00007ffa`f085458d     OpenConsole!ConsoleInputThreadProcWin32+0xa4 [D:\dev\public\terminal\src\interactivity\win32\WindowIo.cpp @ 1053] 
04 00000083`b78ffe60 00007ffa`f1297558     KERNEL32!BaseThreadInitThunk+0x1d [clientcore\base\win32\client\thread.c @ 75] 
05 00000083`b78ffe90 00000000`00000000     ntdll!RtlUserThreadStart+0x28 [minkernel\ntdll\rtlstrt.c @ 1192] 

oh boy. We're crashing here. At this point in startup the VtIo's _pPtySignalInputThread is null, but we're calling VtIo::CreatePseudoWindow, which needs one of those. Let's look at all the stacks.

   0  Id: bc68.af7c Suspend: 1 Teb: 00000083`b7643000 Unfrozen "Console Driver Message IO Thread"
 # Child-SP          RetAddr               Call Site
00 00000083`b7bfe298 00007ffa`ee94919e     ntdll!ZwWaitForSingleObject+0x14 [minkernel\ntdll\daytona\objfre\amd64\usrstubs.asm @ 251] 
01 00000083`b7bfe2a0 00007ff6`f1156113     KERNELBASE!WaitForSingleObjectEx+0x8e [minkernel\kernelbase\synch.c @ 1328] 
02 00000083`b7bfe340 00007ff6`f128395a     OpenConsole!wil::handle_wait+0x33 [D:\dev\public\terminal\packages\Microsoft.Windows.ImplementationLibrary.1.0.220201.1\include\wil\resource.h @ 2601] 
03 00000083`b7bfe390 00007ff6`f1260fd9     OpenConsole!wil::event_t<wil::details::unique_storage<wil::details::resource_policy<void *,void (__cdecl*)(void *) noexcept,&wil::details::CloseHandle,wistd::integral_constant<unsigned __int64,0>,void *,void *,0,std::nullptr_t> >,wil::err_returncode_policy>::wait+0x3a [D:\dev\public\terminal\packages\Microsoft.Windows.ImplementationLibrary.1.0.220201.1\include\wil\resource.h @ 2667] 
04 00000083`b7bfe3c0 00007ff6`f1514e5f     OpenConsole!ConsoleAllocateConsole+0x4b9 [D:\dev\public\terminal\src\host\srvinit.cpp @ 934] 
05 00000083`b7bfe580 00007ff6`f1514591     OpenConsole!IoDispatchers::ConsoleHandleConnectionRequest+0x35f [D:\dev\public\terminal\src\server\IoDispatchers.cpp @ 462] 
06 00000083`b7bff810 00007ff6`f12639de     OpenConsole!IoSorter::ServiceIoOperation+0x101 [D:\dev\public\terminal\src\server\IoSorter.cpp @ 37] 
07 00000083`b7bff890 00007ffa`f085458d     OpenConsole!ConsoleIoThread+0x28e [D:\dev\public\terminal\src\host\srvinit.cpp @ 1059] 
08 00000083`b7bffba0 00007ffa`f1297558     KERNEL32!BaseThreadInitThunk+0x1d [clientcore\base\win32\client\thread.c @ 75] 
09 00000083`b7bffbd0 00000000`00000000     ntdll!RtlUserThreadStart+0x28 [minkernel\ntdll\rtlstrt.c @ 1192] 

   1  Id: bc68.8224 Suspend: 1 Teb: 00000083`b7645000 Unfrozen "Rendering Output Thread"
 # Child-SP          RetAddr               Call Site
00 00000083`b75efdb8 00007ffa`ee94919e     ntdll!ZwWaitForSingleObject+0x14 [minkernel\ntdll\daytona\objfre\amd64\usrstubs.asm @ 251] 
01 00000083`b75efdc0 00007ff6`f11ba391     KERNELBASE!WaitForSingleObjectEx+0x8e [minkernel\kernelbase\synch.c @ 1328] 
02 00000083`b75efe60 00007ff6`f11ba2b2     OpenConsole!Microsoft::Console::Render::RenderThread::_ThreadProc+0xc1 [D:\dev\public\terminal\src\renderer\base\thread.cpp @ 205] 
03 00000083`b75efec0 00007ffa`f085458d     OpenConsole!Microsoft::Console::Render::RenderThread::s_ThreadProc+0x32 [D:\dev\public\terminal\src\renderer\base\thread.cpp @ 160] 
04 00000083`b75eff00 00007ffa`f1297558     KERNEL32!BaseThreadInitThunk+0x1d [clientcore\base\win32\client\thread.c @ 75] 
05 00000083`b75eff30 00000000`00000000     ntdll!RtlUserThreadStart+0x28 [minkernel\ntdll\rtlstrt.c @ 1192] 

#  2  Id: bc68.bb98 Suspend: 1 Teb: 00000083`b7647000 Unfrozen "Win32 Window Message Input Thread"
 # Child-SP          RetAddr               Call Site
00 00000083`b78ffd10 00007ff6`f128b097     OpenConsole!std::optional<Microsoft::Console::PtySignalInputThread::SetParentData>::has_value+0x1b [C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.33.31517\include\optional @ 416] 
01 00000083`b78ffd40 00007ff6`f1294f71     OpenConsole!Microsoft::Console::PtySignalInputThread::CreatePseudoWindow+0x27 [D:\dev\public\terminal\src\host\PtySignalInputThread.cpp @ 90] 
02 00000083`b78ffd80 00007ff6`f116a854     OpenConsole!Microsoft::Console::VirtualTerminal::VtIo::CreatePseudoWindow+0x31 [D:\dev\public\terminal\src\host\VtIo.cpp @ 329] 
03 00000083`b78ffdb0 00007ffa`f085458d     OpenConsole!ConsoleInputThreadProcWin32+0xa4 [D:\dev\public\terminal\src\interactivity\win32\WindowIo.cpp @ 1053] 
04 00000083`b78ffe60 00007ffa`f1297558     KERNEL32!BaseThreadInitThunk+0x1d [clientcore\base\win32\client\thread.c @ 75] 
05 00000083`b78ffe90 00000000`00000000     ntdll!RtlUserThreadStart+0x28 [minkernel\ntdll\rtlstrt.c @ 1192] 

Okay that seems mostly reasonable. There's now just a case where, for conhost --headless, VtIo::CreatePseudoWindow is called before VtIo::CreateAndStartSignalThread. Almost certainly regressed around #13066, though there was a ServiceLocator::LocatePseudoWindow there in ConsoleInputThreadProcWin32 all the way back to the initial source code release, so shit.


This is probably due to there not being a signal handle.

  • Check if we can do the headless thing with just other flags as a heuristic (i.e. --signal). Check sshd, wsl, telnetd, API implementations to make sure.
  • Just figure out a sensible way to launch conhost w/o a signal handle
  • Did this really ever work?

@zadjii-msft
Copy link
Member

Easiest delta to fix this is probably just

void VtIo::CreatePseudoWindow()
{
+    if (_pPtySignalInputThread) {
        _pPtySignalInputThread->CreatePseudoWindow();
+    else {
+        ServiceLocator::LocatePseudoWindow(owner);
+    }
}

@ghost ghost added the In-PR This issue has a related PR label Sep 8, 2022
@ghost ghost closed this as completed in #13950 Sep 8, 2022
ghost pushed a commit that referenced this issue Sep 8, 2022
It's not supported, and it shouldn't be. 

But this is the absolutely most minimal way to get the crash to go away. 

Closes #13914
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Sep 8, 2022
@ghost
Copy link

ghost commented Sep 13, 2022

🎉This issue was addressed in #13950, which has now been successfully released as Windows Terminal Preview v1.16.252.:tada:

Handy links:

@AveYo
Copy link
Author

AveYo commented Oct 3, 2022

This did not trickle down in Windows 11 latest builds and that's where it's most daring.

@carlos-zamora carlos-zamora removed the Needs-Discussion Something that requires a team discussion before we can proceed label Oct 4, 2022
@DHowett
Copy link
Member

DHowett commented Oct 4, 2022

This did not trickle down in Windows 11 latest builds and that's where it's most daring.

Unfortunately, it's not very likely that we will fix this for existing Windows 11 install, either in the coming 22621 update or in a servicing update. It doesn't impact any previously supported user scenarios, and there's no strong business justification for bringing the fix into the coming version of Windows since that version of Windows has already been signed-off-on and sealed.

@AveYo
Copy link
Author

AveYo commented Oct 4, 2022

@DHowett thankfully this only got broken in 11 Dev builds above 25179, so no objective reason not to expect the fix in upcoming 11 Dev builds.

@DHowett
Copy link
Member

DHowett commented Oct 4, 2022

@DHowett thankfully this only got broken in 11 Dev builds above 25179, so no objective reason not to expect the fix in upcoming 11 Dev builds.

Oh, right! In that case, yes, the fix will be available in a future Dev build. We don't migrate the changes made in this repository over to the Windows codebase immediately, but we do migrate them eventually.

@AveYo
Copy link
Author

AveYo commented Sep 5, 2023

Close to a year later and this crap is still not fixed.

@microsoft microsoft locked as resolved and limited conversation to collaborators Sep 5, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants