Bug Report: crashing when Snap app window

[gdamicosmc]

Environment

Windows build number: 10.0.18362.175
Windows Terminal version (if applicable): 0.2.1715.0

Any other software?

Steps to reproduce

Open Windows Terminal, snap window to side or corner of desktop, attempt to interact with Terminal app with no response, then app crashes and closes unexpectedly.

Expected behavior

Snapping Windows Terminal window to corner or side of desktop, then interact with app as usual.

Actual behavior

After snapping app window, app becomes unresponsive, then crashes and closes unexpectedly.

[andrp92]

I can confirm this issue. Is happening to me too. I've got same build

[AzAgarampur]

Strange. I don't have the issue, and I am running the same Windows Terminal Version. Does anyone have a dump from msvsmon.exe when the crash occurs?

[gdamicosmc]

I only have Event Viewer application errors logs.
Errors seen grouped by WindowsTerminal.exe app crash then pwsh.exe .NET Runtime crash. Have PowerShell core as the default profile.
Since first reported, I've had the "2019-05 Cumulative Update for .NET Framework 3.5 and 4.8" update installed and a reboot and now no longer experiencing the issue.

[Shorotshishir]

tried for all terminals within the treminal app, couldn't reproduce the behavior. as u mentioned this probably is causing due to the version of the .Net framework installed in your system machine.
If anyone is having the same problem try Updating .Net Framework your system machine to the latest It should resolve this issue.

[miniksa]

I'm going to need a crash dump here to go further. I tried to reproduce it locally and could not.

If you have the event viewer log of the crash in the Application log with event ID 1001 (Windows Error Reporting) and can give me that, I might be able to find the crash ID in our reporting system and figure this out from that.

[gdamicosmc]

@miniksa I've attached the application logs when the Terminal app was crashing, as requested. There seemed to be a couple of different WER events for the app so they are included. Let me know if you need any thing else.
AppLog-WinTerminalCrash.zip

[AzAgarampur]

So there are mainly 2 problems from the logs I see. PowerShell.exe and pwsh.exe are both crashing due to the console input being redirected somehow, and no processes attached to named/unnamed pipes. And WindowsTerminal.exe is crashing due to access violations. The module is TerminalControl.dll. Can someone use just in time debugging when the crash occurs and drop the VS debugger output?

[miniksa]

See this guy? The hashed bucket number? I looked in the one with WindowsTerminal.exe crashing.

I can find the crash analysis on the back-end with that.

Crash analysis tells me that this is the stack of the failure...

0c (Inline Function) --------`-------- TerminalControl!Microsoft::WRL::ComPtr<ID2D1RenderTarget>::operator->+0x7 [c:\program files (x86)\windows kits\10\include\10.0.17763.0\winrt\wrl\client.h @ 316] 
0d 0000004e`0edff7e0 00007ff8`04d8f892 TerminalControl!Microsoft::Console::Render::DxEngine::StartPaint+0x198 [c:\ba\2009\s\src\renderer\dx\dxrenderer.cpp @ 689] 
0e 0000004e`0edff830 00007ff8`04d8f7da TerminalControl!Microsoft::Console::Render::Renderer::_PaintFrameForEngine+0x72 [c:\ba\2009\s\src\renderer\base\renderer.cpp @ 84] 
0f 0000004e`0edff920 00007ff8`04d8f552 TerminalControl!Microsoft::Console::Render::Renderer::PaintFrame+0x6a [c:\ba\2009\s\src\renderer\base\renderer.cpp @ 65] 
10 (Inline Function) --------`-------- TerminalControl!Microsoft::Console::Render::RenderThread::_ThreadProc+0x40 [c:\ba\2009\s\src\renderer\base\thread.cpp @ 165] 
11 0000004e`0edff950 00007ff8`3db77bd4 TerminalControl!Microsoft::Console::Render::RenderThread::s_ThreadProc+0x52 [c:\ba\2009\s\src\renderer\base\thread.cpp @ 148] 

... checking the DxEngine object in the crash is showing me that _d2dRenderTarget is null at this point.

The line blamed is DxRenderer.cpp @ 689. Which doesn't make sense for a nullptr deref on master, but it does make sense as of this commit time: https://github.com/microsoft/terminal/blob/9b92986b49bed8cc41fde4d6ef080921c41e6d9e/src/renderer/dx/DxRenderer.cpp

Now reasoning this out.... There are three paths above to calling this code.

1. If we don't _haveDeviceResources we called _CreateDeviceResources(true). Looking inside that method, it would have to successfully create and store a _d2dRenderTarget in order to return successfully and then for StartPaint to get down to 689... so it wasn't this.
2. If clientSize changed, we reset the _d2dRenderTarget and then tried to remake it with _PrepareRenderTarget (which is the same method called inside _CreateDeviceResources(true).) At which point, we should have a valid _d2dRenderTarget if we got this far or it would have returned a failure code and we would have bailed.
3. The third route, the else case, which isn't set. We supposedly had device resources and didn't change the size, but the _d2dRenderTarget is gone anyway! How the heck did that happen?

OK. Let's find all places where the contents of _d2dRenderTarget is modified.

1. Line 263, creation of the render target in _PrepareRenderTarget
2. Line 308, reset of the smart pointer when _ReleaseDeviceResources is called
3. Line 683, reset of the smart pointer when ResizeBuffers-ing the _dxgiSwapChain during StartPaint.

For Line 263, _PrepareRenderTarget appears to be always wrapped in a result code checker and so are the methods that use it. So I don't think it's that one.

For line 308, _ReleaseDeviceResources first operation is to set _haveDeviceResources to false. But if it got to this one and _haveDeviceResources was false, then we wouldn't have hit the third route above. We would have created them first and errored if they weren't created. _haveDeviceResources isn't set to true until the bottom of _CreateDeviceResources after all the early return errors. So the target should have existed.

For line 683, this is among the newest things going on. I added this code more recently than the other bits to attempt to resize without unspooling the entire device resources pipeline and recreating it because it was expensive (and was probably causing some sort of flickering or performance nightmare. I forget exactly, but it had to be something like that.)

So I'm going to suspect line 683 here or the nearby code.

Reasoning this out... I'm first concerned because the ->ResizeBuffers call is returning an HRESULT and it's not being checked in any way. This means if it fails, we're not reporting it.

Secondly, I'm detaching the _d2dRenderTarget on 683 then attempting to resize and recreate it while leaving the state of _haveDeviceResources as true. If line 685 threw an error and returned (because it couldn't create the new _d2dRenderTarget... then we could end up in a torn state. The _d2dRenderTarget was reset but the boolean still seems to think it exists.

That sounds very plausible.

So what I'm going to do here is go on the assumption that this is the problem. I don't want to "throw out the baby with the bathwater" by undoing this change which must have been for good reason. I'm just going to try to make it more robust.

I'll set up a situation where:

1. We check ->ResizeBuffers for a failure because it probably should be and RETURN_IF_FAILED if it is bad.
2. If ResizeBuffers or _PrepareRenderTarget fails while we're in this somewhat torn state (where I only released part of the device resources but haven't set _haveDeviceResources to false yet), I'll call _ReleaseDeviceResources on the way out to make sure everything gets cleaned up and is recreated from new the next time around.

I'm not certain exactly what the problem is while creating the render target, but this proposal should stop the crash from happening by eliminating the torn state.

After that, if there's still weirdness when snapping the window, we should be able to reproduce it under the debugger and see what error codes are being thrown by DirectX (because the RETURN_IF and LOG_IF macros should be reporting those failures when attached to a debugger in the output window) and go from there.

[DHowett-MSFT]

Thanks again for the report! This was just submitted to the store with the v0.2.1831.0 servicing release. It may take some time for the store to process it.

[gdamicosmc]

@DHowett-MSFT thanks for the update, that's great!