Skip to content

Commit

Permalink
Fix crash when closing panes very quickly (#17450)
Browse files Browse the repository at this point in the history
#17358 introduced a bug where if you open/close panes very rapidly
Terminal will crash. This was because `_content` was being set to `null`
and then a `Close` event was being emitted, but several functions
attempt to access the pane's `_content` as part of the close routine.
For example, `TerminalTab` tries to update the `TaskbarProgress` every
time a pane is closed and as part of that update sequence it queries the
pane - which has `null` content now - for the taskbar progress,
resulting in a crash. This PR fixes that crash.

Refs #17358
  • Loading branch information
PankajBhojwani authored Jun 20, 2024
1 parent a80539c commit 5d46e31
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1711,7 +1711,9 @@ void Pane::_SetupChildCloseHandlers()
IPaneContent Pane::_takePaneContent()
{
_closeRequestedRevoker.revoke();
return std::move(_content);
// we cannot return std::move(_content) because we don't want _content to be null,
// since _content gets accessed even after Close is called
return _content;
}

// This method safely sets the content of the Pane. It'll ensure to revoke and
Expand All @@ -1721,15 +1723,14 @@ void Pane::_setPaneContent(IPaneContent content)
{
// The IPaneContent::Close() implementation may be buggy and raise the CloseRequested event again.
// _takePaneContent() avoids this as it revokes the event handler.
if (const auto c = _takePaneContent())
if (_takePaneContent())
{
c.Close();
_content.Close();
}

_content = std::move(content);

if (_content)
if (content)
{
_content = std::move(content);
_closeRequestedRevoker = _content.CloseRequested(winrt::auto_revoke, [this](auto&&, auto&&) { Close(); });
}
}
Expand Down

0 comments on commit 5d46e31

Please sign in to comment.