Closed DHowett closed 2 months ago
note: We greatly reduced the number of places where this can trigger in #13352, but this quirk isn't entirely gone, so this bug persists.
fwiw I think I hit the same issue doing the following:
It results in:
> Microsoft.Terminal.Control.dll!Microsoft::Terminal::Core::Terminal::Write(std::basic_string_view<wchar_t,std::char_traits<wchar_t>> stringView) Line 486 C++
Microsoft.Terminal.Control.dll!winrt::Microsoft::Terminal::Control::implementation::ControlCore::_connectionOutputHandler(const winrt::hstring & hstr) Line 1515 C++
[Inline Frame] Microsoft.Terminal.Control.dll!winrt::Microsoft::Terminal::TerminalConnection::TerminalOutputHandler::{ctor}::__l1::<lambda_212_>::operator()(const winrt::hstring &) Line 507 C++
Microsoft.Terminal.Control.dll!winrt::impl::delegate<winrt::Microsoft::Terminal::TerminalConnection::TerminalOutputHandler,`winrt::Microsoft::Terminal::TerminalConnection::TerminalOutputHandler::implementation<winrt::Microsoft::Terminal::Control::implementation::ControlCore,void (__cdecl winrt::Microsoft::Terminal::Control::implementation::ControlCore::*)(winrt::hstring const &)>'::`1'::<lambda_212_>>::Invoke(void * output) Line 174 C++
[Inline Frame] TerminalConnection.dll!winrt::Microsoft::Terminal::TerminalConnection::TerminalOutputHandler::operator()(const winrt::param::hstring &) Line 451 C++
TerminalConnection.dll!winrt::impl::invoke<winrt::Microsoft::Terminal::TerminalConnection::TerminalOutputHandler,std::wstring>(const winrt::Microsoft::Terminal::TerminalConnection::TerminalOutputHandler & delegate, const std::wstring & <args_0>) Line 5762 C++
[Inline Frame] TerminalConnection.dll!winrt::event<winrt::Microsoft::Terminal::TerminalConnection::TerminalOutputHandler>::operator()(const std::wstring &) Line 5897 C++
TerminalConnection.dll!winrt::Microsoft::Terminal::TerminalConnection::implementation::ConptyConnection::_OutputThread() Line 659 C++
[Inline Frame] TerminalConnection.dll!winrt::Microsoft::Terminal::TerminalConnection::implementation::ConptyConnection::Start::__l3::<lambda_1>::operator()(void *) Line 362 C++
TerminalConnection.dll!_Closure_wrapper_1bfe9aa2_14::<lambda_invoker_cdecl>(void * __p1) Line 368 C++
kernel32.dll!BaseThreadInitThunk() Unknown
ntdll.dll!RtlUserThreadStart() Unknown
| Name | Value | Type
-- | -- | -- | --
▶ | this | 0x000002317a8988a0 {_pfnWriteInput={__this=0x000002317d41adc0 {...} } _pfnWarningBell=bind(0x00007fff6542dcd0 {Microsoft.Terminal.Control.dll!winrt::Microsoft::Terminal::Control::implementation::ControlCore::_terminalWarningBell(void)}, 0x000002317d41adc0 {...}) ...} | Microsoft::Terminal::Core::Terminal *
| cursorPosAfter | Variable is optimized away and not available. |
▶ | cursorPosBefore | {x=0 X=0 y=32 ...} | til::point
▶ | lock | {_Pmtx=0x000002317a8989c0 {_next_ticket=435 _now_serving=434 } _Owns=true } | std::unique_lock\x1b[?1049
appearing in the stringView
seems suspicious.
This happens somewhat frequently while using WeeChat normally (i.e. not just looking at the greeting screen), so it's quite annoying, and has been occurring for a while.
I'm currently on Terminal 1.14.2282.0
I've just intsalled Microsoft.WindowsTerminal_Win11_1.15.2525.0_8wekyb3d8bbwe.msixbundle (hopefully that doesn't screw up the package on my system, which was previously just installed via Store...) and the crash is still present.
Whoa, thanks for this! A repro that doesn't involve the powershell quirk (which we just removed!) makes this much more important, too.
I was going to take a crack at fixing this (at least locally), and I find that on my local build of current main branch (commit 1f19ed0cd2e04dd46852300c369960d6fa69a8fa), the crash does not occur. On the current release version (1.15.2875.0) I have installed, the crash still does occur.
What's the likelyhood this was actually fixed as opposed to being fixed by some difference in the build environment? What commit does 1.15.2875.0 relate to? There is no matching git tag.
Strangely I started getting a crash-on-resize again after running a round of Windows Update today (not sure if crash has same root cause though), and doing a clean rebuild of Terminal locally seems to have made the crash go away, again. I also updated VS2022 to current latest, so a lot of moving pieces, but... 🤨
Okay, coming back to this thread now that I'm back from leave and vacation.
I can't seem to repro this with weechat in main
anymore. There've been some locking and lifetime changes over the last few months that might have fixed this. @shuffle2 if you can't repro this on main
anymore, then I suggest we punt this out to the backlog. The set of folks with the busted PsReadline that we still need to quirk for who are resizing their terminal while entering the alt buffer is gonna be samll and only shrinking. We can elevate from the backlog if we see live hits here, but I don't think there are any.
Actually heck, @shuffle2's crash is a different thing anyways. The pwsh VT quirk only applies to conhost, and that stack is firmly in terminal code. I'm gonna punt and assume this was fixed. I vaguely recall some other resizing crash fixes, but 1.17 has been like, 4 months now.
@zadjii-msft I've just updated Windows and Terminal (from the store) again, and my crash (crash when resizing weechat output) still repros. The versions are: Terminal 1.18.2822.0 Windows 10.0.22621.2428
Although, getting the crash to occur isn't very deterministic. It seems to crash less frequently than before :/ Might need to try and compile with some debugging functionality and track it down eventually...unfortunately not much of a helpful update from me, here.
edit: actually, I'm probably hitting yet another crash, just happens to also be while doing my standard "resize weechat test". The current crash which has happened a few times looks like this:
Repro:
Resize the window horizontally while it's running.
Toggling the alt buffer from pwsh.exe or powershell.exe (specifically) causes us to UAF the outgoing
SCREEN_INFORMATION
here: https://github.com/microsoft/terminal/blob/71cbdc8a1f6b8da10d130c053489445e99a4fc13/src/host/_stream.cpp#L1038-L1040