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

Tab teardown crashing resulting from renewed renderer destruction order issues #1198

Closed
DHowett-MSFT opened this issue Jun 10, 2019 · 1 comment · Fixed by #1199
Closed
Assignees
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news.

Comments

@DHowett-MSFT
Copy link
Contributor

DHowett-MSFT commented Jun 10, 2019

I can reliably crash a release build of WT by just closing tabs.

>	TerminalControl.dll!Microsoft::Console::Render::DxEngine::_CreateDeviceResources(const bool) Line 170	C++
 	TerminalControl.dll!Microsoft::Console::Render::DxEngine::StartPaint() Line 692	C++
 	TerminalControl.dll!Microsoft::Console::Render::Renderer::_PaintFrameForEngine(Microsoft::Console::Render::IRenderEngine * const pEngine) Line 90	C++
 	TerminalControl.dll!Microsoft::Console::Render::Renderer::PaintFrame() Line 68	C++
 	[Inline Frame] TerminalControl.dll!Microsoft::Console::Render::RenderThread::_ThreadProc()	C++
 	TerminalControl.dll!Microsoft::Console::Render::RenderThread::s_ThreadProc(void * lpParameter) Line 151	C++

Various things under RenderThread::s_ThreadProc.

If you ask the Renderer what it thinks, it says this:

this->_destructing
true

However, we're here:

HRESULT Renderer::PaintFrame()
{
if (_destructing)
{
return S_FALSE;
}
for (IRenderEngine* const pEngine : _rgpEngines)
{
LOG_IF_FAILED(_PaintFrameForEngine(pEngine));
}
return S_OK;

We passed the _destructing check and . . . now we're very sad.

@DHowett-MSFT DHowett-MSFT added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Severity-Crash Crashes are real bad news. Product-Terminal The new Windows Terminal. labels Jun 10, 2019
@DHowett-MSFT DHowett-MSFT added this to the Terminal v0.2 milestone Jun 10, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 10, 2019
@DHowett-MSFT
Copy link
Contributor Author

This seems worse since #825.

@DHowett-MSFT DHowett-MSFT self-assigned this Jun 11, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 11, 2019
DHowett-MSFT pushed a commit that referenced this issue Jun 11, 2019
This commit introduces a few automatic event revokers and implements
staged Close for TermControl's constituent components.

Fixes #1198.
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Jun 11, 2019
DHowett-MSFT pushed a commit that referenced this issue Jun 11, 2019
* Properly manage teardown state in TermControl

This commit introduces a few automatic event revokers and implements
staged Close for TermControl's constituent components.

* Only read the focused control title if there is one

Fixes #1198
Fixes #1188
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant