Skip to content

Commit

Permalink
AtlasEngine: Fix leak check report for OpenConsole (#12415)
Browse files Browse the repository at this point in the history
AtlasEngine enables various debug options for D2D and D3D in Debug builds.
Among those are resource leak checks, which were broken in OpenConsole
due to the unclean exit in `ServiceLocator::RundownAndExit`.
This commit fixes the issue by running the destructors
of any renderers registered in the Window class first.

## PR Checklist
* [x] Closes #12414
* [x] I work here

## Validation Steps Performed

* Set `HKEY_CURRENT_USER\Console\UseDx` to `2`
* Run `OpenConsole.exe`
* Exit
* No exceptions are thrown ✅
  • Loading branch information
lhecker authored Feb 11, 2022
1 parent 8b90669 commit 9d7a46f
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 24 deletions.
18 changes: 10 additions & 8 deletions src/interactivity/base/ServiceLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ using namespace Microsoft::Console::Interactivity;
std::unique_ptr<IInteractivityFactory> ServiceLocator::s_interactivityFactory;
std::unique_ptr<IConsoleControl> ServiceLocator::s_consoleControl;
std::unique_ptr<IConsoleInputThread> ServiceLocator::s_consoleInputThread;
std::unique_ptr<IConsoleWindow> ServiceLocator::s_consoleWindow;
std::unique_ptr<IWindowMetrics> ServiceLocator::s_windowMetrics;
std::unique_ptr<IAccessibilityNotifier> ServiceLocator::s_accessibilityNotifier;
std::unique_ptr<IHighDpiApi> ServiceLocator::s_highDpiApi;
std::unique_ptr<ISystemConfigurationProvider> ServiceLocator::s_systemConfigurationProvider;
std::unique_ptr<IInputServices> ServiceLocator::s_inputServices;

IConsoleWindow* ServiceLocator::s_consoleWindow = nullptr;

Globals ServiceLocator::s_globals;

bool ServiceLocator::s_pseudoWindowInitialized = false;
Expand All @@ -45,6 +44,10 @@ wil::unique_hwnd ServiceLocator::s_pseudoWindow = nullptr;
s_globals.pRender->TriggerTeardown();
}

// By locking the console, we ensure no background tasks are accessing the
// classes we're going to destruct down below (for instance: CursorBlinker).
s_globals.getConsoleInformation().LockConsole();

// A History Lesson from MSFT: 13576341:
// We introduced RundownAndExit to give services that hold onto important handles
// an opportunity to let those go when we decide to exit from the console for various reasons.
Expand All @@ -62,10 +65,9 @@ wil::unique_hwnd ServiceLocator::s_pseudoWindow = nullptr;

// TODO: MSFT: 14397093 - Expand graceful rundown beyond just the Hot Bug input services case.

if (s_inputServices.get() != nullptr)
{
s_inputServices.reset(nullptr);
}
delete s_globals.pRender;
s_inputServices.reset(nullptr);
s_consoleWindow.reset(nullptr);

ExitProcess(hr);
}
Expand Down Expand Up @@ -154,7 +156,7 @@ wil::unique_hwnd ServiceLocator::s_pseudoWindow = nullptr;
}
else
{
s_consoleWindow = window;
s_consoleWindow.reset(window);
}

return status;
Expand All @@ -166,7 +168,7 @@ wil::unique_hwnd ServiceLocator::s_pseudoWindow = nullptr;

IConsoleWindow* ServiceLocator::LocateConsoleWindow()
{
return s_consoleWindow;
return s_consoleWindow.get();
}

IConsoleControl* ServiceLocator::LocateConsoleControl()
Expand Down
4 changes: 2 additions & 2 deletions src/interactivity/inc/ServiceLocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ namespace Microsoft::Console::Interactivity
template<typename T>
static T* LocateConsoleWindow()
{
return static_cast<T*>(s_consoleWindow);
return static_cast<T*>(s_consoleWindow.get());
}

static IWindowMetrics* LocateWindowMetrics();
Expand Down Expand Up @@ -106,7 +106,7 @@ namespace Microsoft::Console::Interactivity
// TODO: MSFT 15344939 - some implementations of IConsoleWindow are currently singleton
// classes so we can't own a pointer to them here. fix this so s_consoleWindow can follow the
// pattern of the rest of the service interface pointers.
static IConsoleWindow* s_consoleWindow;
static std::unique_ptr<IConsoleWindow> s_consoleWindow;
static std::unique_ptr<IWindowMetrics> s_windowMetrics;
static std::unique_ptr<IHighDpiApi> s_highDpiApi;
static std::unique_ptr<ISystemConfigurationProvider> s_systemConfigurationProvider;
Expand Down
20 changes: 7 additions & 13 deletions src/interactivity/win32/window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ using namespace Microsoft::Console::Interactivity;
using namespace Microsoft::Console::Render;

ATOM Window::s_atomWindowClass = 0;
Window* Window::s_Instance = nullptr;

Window::Window() :
_fIsInFullscreen(false),
Expand All @@ -68,10 +67,13 @@ Window::Window() :

Window::~Window()
{
if (ServiceLocator::LocateGlobals().pRender != nullptr)
{
delete ServiceLocator::LocateGlobals().pRender;
}
delete pGdiEngine;
#if TIL_FEATURE_CONHOSTDXENGINE_ENABLED
delete pDxEngine;
#endif
#if TIL_FEATURE_ATLASENGINE_ENABLED
delete pAtlasEngine;
#endif
}

// Routine Description:
Expand All @@ -98,7 +100,6 @@ Window::~Window()

if (NT_SUCCESS(status))
{
Window::s_Instance = pNewWindow;
LOG_IF_FAILED(ServiceLocator::SetConsoleWindowInstance(pNewWindow));
}
}
Expand Down Expand Up @@ -210,13 +211,6 @@ void Window::_UpdateSystemMetrics() const
_UpdateSystemMetrics();

const auto useDx = pSettings->GetUseDx();
GdiEngine* pGdiEngine = nullptr;
#if TIL_FEATURE_CONHOSTDXENGINE_ENABLED
DxEngine* pDxEngine = nullptr;
#endif
#if TIL_FEATURE_ATLASENGINE_ENABLED
AtlasEngine* pAtlasEngine = nullptr;
#endif
try
{
switch (useDx)
Expand Down
16 changes: 15 additions & 1 deletion src/interactivity/win32/window.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ Author(s):

#include "../inc/IConsoleWindow.hpp"

namespace Microsoft::Console::Render
{
class AtlasEngine;
class DxEngine;
class GdiEngine;
}

namespace Microsoft::Console::Interactivity::Win32
{
class WindowUiaProvider;
Expand Down Expand Up @@ -105,7 +112,14 @@ namespace Microsoft::Console::Interactivity::Win32
Settings* _pSettings;

HWND _hWnd;
static Window* s_Instance;

Render::GdiEngine* pGdiEngine = nullptr;
#if TIL_FEATURE_CONHOSTDXENGINE_ENABLED
Render::DxEngine* pDxEngine = nullptr;
#endif
#if TIL_FEATURE_ATLASENGINE_ENABLED
Render::AtlasEngine* pAtlasEngine = nullptr;
#endif

[[nodiscard]] NTSTATUS _InternalSetWindowSize();
void _UpdateWindowSize(const SIZE sizeNew);
Expand Down

0 comments on commit 9d7a46f

Please sign in to comment.