From 9ad2544033649077a3fc3be487aa5799655c1dc7 Mon Sep 17 00:00:00 2001
From: Michael Ratanapintha <1039862+metathinker@users.noreply.github.com>
Date: Tue, 28 May 2019 09:56:36 -0700
Subject: [PATCH] Fix #936: misuse of uninitialized objects causes AppVerifier
 breaks on Windows Terminal startup (#1015)

* move the render thread init up; gets rid of verifier stops

* s/INVALID_HANDLE_VALUE/NULL/g since CreateEvent() and CreateThread() return a NULL HANDLE on failure; resolves another cause of AppVerifier breaks
---
 src/cascadia/TerminalControl/TermControl.cpp | 12 ++++++----
 src/renderer/base/thread.cpp                 | 24 ++++++++++----------
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp
index 0eacbc9bb..0ee637398 100644
--- a/src/cascadia/TerminalControl/TermControl.cpp
+++ b/src/cascadia/TerminalControl/TermControl.cpp
@@ -297,13 +297,19 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         _terminal = std::make_unique<::Microsoft::Terminal::Core::Terminal>();
 
         // First create the render thread.
+        // Then stash a local pointer to the render thread so we can initialize it and enable it
+        // to paint itself *after* we hand off its ownership to the renderer.
+        // We split up construction and initialization of the render thread object this way
+        // because the renderer and render thread have circular references to each other.
         auto renderThread = std::make_unique<::Microsoft::Console::Render::RenderThread>();
-        // Stash a local pointer to the render thread, so we can enable it after
-        //       we hand off ownership to the renderer.
         auto* const localPointerToThread = renderThread.get();
+
+        // Now create the renderer and initialize the render thread.
         _renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(_terminal.get(), nullptr, 0, std::move(renderThread));
         ::Microsoft::Console::Render::IRenderTarget& renderTarget = *_renderer;
 
+        THROW_IF_FAILED(localPointerToThread->Initialize(_renderer.get()));
+
         // Set up the DX Engine
         auto dxEngine = std::make_unique<::Microsoft::Console::Render::DxEngine>();
         _renderer->AddRenderEngine(dxEngine.get());
@@ -350,8 +356,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         auto inputFn = std::bind(&TermControl::_SendInputToConnection, this, std::placeholders::_1);
         _terminal->SetWriteInputCallback(inputFn);
 
-        THROW_IF_FAILED(localPointerToThread->Initialize(_renderer.get()));
-
         auto chain = _renderEngine->GetSwapChain();
         _swapChainPanel.Dispatcher().RunAsync(CoreDispatcherPriority::High, [this, chain]()
         {
diff --git a/src/renderer/base/thread.cpp b/src/renderer/base/thread.cpp
index 80a398768..4c7fc0e1b 100644
--- a/src/renderer/base/thread.cpp
+++ b/src/renderer/base/thread.cpp
@@ -12,42 +12,42 @@ using namespace Microsoft::Console::Render;
 
 RenderThread::RenderThread() :
     _pRenderer(nullptr),
-    _hThread(INVALID_HANDLE_VALUE),
-    _hEvent(INVALID_HANDLE_VALUE),
-    _hPaintCompletedEvent(INVALID_HANDLE_VALUE),
+    _hThread(nullptr),
+    _hEvent(nullptr),
+    _hPaintCompletedEvent(nullptr),
     _fKeepRunning(true),
-    _hPaintEnabledEvent(INVALID_HANDLE_VALUE)
+    _hPaintEnabledEvent(nullptr)
 {
 
 }
 
 RenderThread::~RenderThread()
 {
-    if (_hThread != INVALID_HANDLE_VALUE)
+    if (_hThread)
     {
         _fKeepRunning = false; // stop loop after final run
         SignalObjectAndWait(_hEvent, _hThread, INFINITE, FALSE); // signal final paint and wait for thread to finish.
 
         CloseHandle(_hThread);
-        _hThread = INVALID_HANDLE_VALUE;
+        _hThread = nullptr;
     }
 
-    if (_hEvent != INVALID_HANDLE_VALUE)
+    if (_hEvent)
     {
         CloseHandle(_hEvent);
-        _hEvent = INVALID_HANDLE_VALUE;
+        _hEvent = nullptr;
     }
 
-    if (_hPaintEnabledEvent != INVALID_HANDLE_VALUE)
+    if (_hPaintEnabledEvent)
     {
         CloseHandle(_hPaintEnabledEvent);
-        _hPaintEnabledEvent = INVALID_HANDLE_VALUE;
+        _hPaintEnabledEvent = nullptr;
     }
 
-    if (_hPaintCompletedEvent != INVALID_HANDLE_VALUE)
+    if (_hPaintCompletedEvent)
     {
         CloseHandle(_hPaintCompletedEvent);
-        _hPaintCompletedEvent = INVALID_HANDLE_VALUE;
+        _hPaintCompletedEvent = nullptr;
     }
 }