From 6d0342f0bb31bf245843411c6781d6d5399ff651 Mon Sep 17 00:00:00 2001
From: Leonard Hecker <lhecker@microsoft.com>
Date: Tue, 7 May 2024 20:35:48 +0200
Subject: [PATCH] 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.
---
 src/cascadia/TerminalControl/ControlCore.cpp |  4 ++--
 src/cascadia/TerminalControl/TermControl.cpp | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
index da78e6167a1..575864ede1a 100644
--- a/src/cascadia/TerminalControl/ControlCore.cpp
+++ b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -161,7 +161,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             std::chrono::milliseconds{ 100 },
             [weakTerminal = std::weak_ptr{ _terminal }, weakThis = get_weak(), dispatcher = _dispatcher]() {
                 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);
                     }
@@ -179,7 +179,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             _dispatcher,
             std::chrono::milliseconds{ 8 },
             [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);
                 }
diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp
index df1e15164aa..c1c7cc73e53 100644
--- a/src/cascadia/TerminalControl/TermControl.cpp
+++ b/src/cascadia/TerminalControl/TermControl.cpp
@@ -248,7 +248,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             dispatcher,
             TerminalWarningBellInterval,
             [weakThis = get_weak()]() {
-                if (auto control{ weakThis.get() }; !control->_IsClosing())
+                if (auto control{ weakThis.get() }; control && !control->_IsClosing())
                 {
                     control->WarningBell.raise(*control, nullptr);
                 }
@@ -258,7 +258,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             dispatcher,
             ScrollBarUpdateInterval,
             [weakThis = get_weak()](const auto& update) {
-                if (auto control{ weakThis.get() }; !control->_IsClosing())
+                if (auto control{ weakThis.get() }; control && !control->_IsClosing())
                 {
                     control->_throttledUpdateScrollbar(update);
                 }
@@ -301,7 +301,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             _originalSelectedSecondaryElements.Append(e);
         }
         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() };
                 menu.PrimaryCommands().Clear();
@@ -317,7 +317,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             }
         });
         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() };
                 menu.PrimaryCommands().Clear();
@@ -544,7 +544,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 }
 
                 _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->_refreshSearch();