Add nullptr checks to shared_ptr conversions (#17199)

We use `if (auto self = weakSelf.get())` in a lot of places.
That assigns the value to `self` and then checks if it's truthy.
Sometimes we need to add a "is (app) closing" check because XAML,
so we wrote something akin to `if (self = ...; !closing)`.

But that's wrong because the correct `if (foo)` is the same as
`if (void; foo)` and not `if (foo; void)` and that meant that
we didn't check for `self`'s truthiness anymore.

This issue became apparent now, because we added a new kind of
delayed callback invocation (which is a lot cheaper).
This made the lack of a `nullptr` check finally obvious.
This commit is contained in:
Leonard Hecker 2024-05-07 20:35:48 +02:00 committed by GitHub
parent 9c16c5ca82
commit 6d0342f0bb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 7 additions and 7 deletions

View File

@ -161,7 +161,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
std::chrono::milliseconds{ 100 }, std::chrono::milliseconds{ 100 },
[weakTerminal = std::weak_ptr{ _terminal }, weakThis = get_weak(), dispatcher = _dispatcher]() { [weakTerminal = std::weak_ptr{ _terminal }, weakThis = get_weak(), dispatcher = _dispatcher]() {
dispatcher.TryEnqueue(DispatcherQueuePriority::Normal, [weakThis]() { dispatcher.TryEnqueue(DispatcherQueuePriority::Normal, [weakThis]() {
if (const auto self = weakThis.get(); !self->_IsClosing()) if (const auto self = weakThis.get(); self && !self->_IsClosing())
{ {
self->OutputIdle.raise(*self, nullptr); self->OutputIdle.raise(*self, nullptr);
} }
@ -179,7 +179,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_dispatcher, _dispatcher,
std::chrono::milliseconds{ 8 }, std::chrono::milliseconds{ 8 },
[weakThis = get_weak()](const auto& update) { [weakThis = get_weak()](const auto& update) {
if (auto core{ weakThis.get() }; !core->_IsClosing()) if (auto core{ weakThis.get() }; core && !core->_IsClosing())
{ {
core->ScrollPositionChanged.raise(*core, update); core->ScrollPositionChanged.raise(*core, update);
} }

View File

@ -248,7 +248,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
dispatcher, dispatcher,
TerminalWarningBellInterval, TerminalWarningBellInterval,
[weakThis = get_weak()]() { [weakThis = get_weak()]() {
if (auto control{ weakThis.get() }; !control->_IsClosing()) if (auto control{ weakThis.get() }; control && !control->_IsClosing())
{ {
control->WarningBell.raise(*control, nullptr); control->WarningBell.raise(*control, nullptr);
} }
@ -258,7 +258,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
dispatcher, dispatcher,
ScrollBarUpdateInterval, ScrollBarUpdateInterval,
[weakThis = get_weak()](const auto& update) { [weakThis = get_weak()](const auto& update) {
if (auto control{ weakThis.get() }; !control->_IsClosing()) if (auto control{ weakThis.get() }; control && !control->_IsClosing())
{ {
control->_throttledUpdateScrollbar(update); control->_throttledUpdateScrollbar(update);
} }
@ -301,7 +301,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_originalSelectedSecondaryElements.Append(e); _originalSelectedSecondaryElements.Append(e);
} }
ContextMenu().Closed([weakThis = get_weak()](auto&&, auto&&) { ContextMenu().Closed([weakThis = get_weak()](auto&&, auto&&) {
if (auto control{ weakThis.get() }; !control->_IsClosing()) if (auto control{ weakThis.get() }; control && !control->_IsClosing())
{ {
const auto& menu{ control->ContextMenu() }; const auto& menu{ control->ContextMenu() };
menu.PrimaryCommands().Clear(); menu.PrimaryCommands().Clear();
@ -317,7 +317,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
} }
}); });
SelectionContextMenu().Closed([weakThis = get_weak()](auto&&, auto&&) { SelectionContextMenu().Closed([weakThis = get_weak()](auto&&, auto&&) {
if (auto control{ weakThis.get() }; !control->_IsClosing()) if (auto control{ weakThis.get() }; control && !control->_IsClosing())
{ {
const auto& menu{ control->SelectionContextMenu() }; const auto& menu{ control->SelectionContextMenu() };
menu.PrimaryCommands().Clear(); menu.PrimaryCommands().Clear();
@ -544,7 +544,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
} }
_searchBox->Open([weakThis = get_weak()]() { _searchBox->Open([weakThis = get_weak()]() {
if (const auto self = weakThis.get(); !self->_IsClosing()) if (const auto self = weakThis.get(); self && !self->_IsClosing())
{ {
self->_searchBox->SetFocusOnTextbox(); self->_searchBox->SetFocusOnTextbox();
self->_refreshSearch(); self->_refreshSearch();