Skip to content

Commit

Permalink
Update wil. Fixes GDI handle leak (#6229)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
When resizing the window title, a GDI object would be leaked. This has to do with our island message handler using `wil` to track these objects and `wil` having a bug.

## References
microsoft/wil#100

## PR Checklist
* [x] Closes #5949
* [x] I work here.
* [x] Tested manually
* [x] Doc not required.
* [x] Am core contributor.

## Validation Steps Performed
* [x] Added the GDI Objects column to Task Manager, set the Terminal to use the `titleWidth` size tabs, then changed the title a bunch with PowerShell. Confirmed repro before (increasing GDI count). Confirmed it's gone after (no change to object count).

(cherry picked from commit 48b3262)
  • Loading branch information
miniksa authored and DHowett committed Jun 24, 2020
1 parent ece96f5 commit c6fc68e
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 4 deletions.
Empty file.
2 changes: 1 addition & 1 deletion src/buffer/out/CharRow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ COORD CharRow::GetStorageKey(const size_t column) const noexcept
// - Updates the pointer to the parent row (which might change if we shuffle the rows around)
// Arguments:
// - pParent - Pointer to the parent row
void CharRow::UpdateParent(ROW* const pParent) noexcept
void CharRow::UpdateParent(ROW* const pParent)
{
_pParent = FAIL_FAST_IF_NULL(pParent);
}
2 changes: 1 addition & 1 deletion src/buffer/out/CharRow.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class CharRow final
const UnicodeStorage& GetUnicodeStorage() const noexcept;
COORD GetStorageKey(const size_t column) const noexcept;

void UpdateParent(ROW* const pParent) noexcept;
void UpdateParent(ROW* const pParent);

friend CharRowCellReference;
friend constexpr bool operator==(const CharRow& a, const CharRow& b) noexcept;
Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/PublicTerminalCore/HwndTerminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ LRESULT CALLBACK HwndTerminal::HwndTerminalWndProc(
UINT uMsg,
WPARAM wParam,
LPARAM lParam) noexcept
try
{
#pragma warning(suppress : 26490) // Win32 APIs can only store void*, have to use reinterpret_cast
HwndTerminal* terminal = reinterpret_cast<HwndTerminal*>(GetWindowLongPtr(hwnd, GWLP_USERDATA));
Expand Down Expand Up @@ -84,6 +85,7 @@ LRESULT CALLBACK HwndTerminal::HwndTerminalWndProc(
}
return DefWindowProc(hwnd, uMsg, wParam, lParam);
}
CATCH_LOG()

static bool RegisterTermClass(HINSTANCE hInstance) noexcept
{
Expand Down Expand Up @@ -686,6 +688,7 @@ void __stdcall TerminalKillFocus(void* terminal)
// - rows - Rows of text data to copy
// - fAlsoCopyFormatting - true if the color and formatting should also be copied, false otherwise
HRESULT HwndTerminal::_CopyTextToSystemClipboard(const TextBuffer::TextAndColor& rows, bool const fAlsoCopyFormatting)
try
{
std::wstring finalString;

Expand Down Expand Up @@ -714,7 +717,7 @@ HRESULT HwndTerminal::_CopyTextToSystemClipboard(const TextBuffer::TextAndColor&
RETURN_LAST_ERROR_IF(!OpenClipboard(_hwnd.get()));

{ // Clipboard Scope
auto clipboardCloser = wil::scope_exit([]() noexcept {
auto clipboardCloser = wil::scope_exit([]() {
LOG_LAST_ERROR_IF(!CloseClipboard());
});

Expand Down Expand Up @@ -742,6 +745,7 @@ HRESULT HwndTerminal::_CopyTextToSystemClipboard(const TextBuffer::TextAndColor&

return S_OK;
}
CATCH_RETURN()

// Routine Description:
// - Copies the given string onto the global system clipboard in the specified format
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/TerminalConnection/ConptyConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
// Method Description:
// - called when the client application (not necessarily its pty) exits for any reason
void ConptyConnection::_ClientTerminated() noexcept
try
{
if (_isStateAtOrBeyond(ConnectionState::Closing))
{
Expand Down Expand Up @@ -321,6 +322,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation

_piClient.reset();
}
CATCH_LOG()

void ConptyConnection::WriteInput(hstring const& data)
{
Expand Down Expand Up @@ -349,6 +351,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
}

void ConptyConnection::Close() noexcept
try
{
if (_transitionToState(ConnectionState::Closing))
{
Expand Down Expand Up @@ -378,6 +381,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
_transitionToState(ConnectionState::Closed);
}
}
CATCH_LOG()

DWORD ConptyConnection::_OutputThread()
{
Expand Down
2 changes: 2 additions & 0 deletions src/renderer/dx/DxRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2236,6 +2236,7 @@ void DxEngine::SetAntialiasingMode(const D2D1_TEXT_ANTIALIAS_MODE antialiasingMo
// Return Value:
// - <none>
void DxEngine::SetDefaultTextBackgroundOpacity(const float opacity) noexcept
try
{
_defaultTextBackgroundOpacity = opacity;

Expand All @@ -2244,3 +2245,4 @@ void DxEngine::SetDefaultTextBackgroundOpacity(const float opacity) noexcept
// We don't terribly care if this fails.
LOG_IF_FAILED(InvalidateAll());
}
CATCH_LOG()

0 comments on commit c6fc68e

Please sign in to comment.