microsoft/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2025-12-10 18:43:54 -06:00
Code Issues Packages Projects Releases 138 Wiki Activity

Fix pane event handlers being unbound (#17750)

Browse Source 
I don't know what has changed between #17450 and now, but that fix
doesn't seem necessary anymore. If you add this action:
```json
{
    "keys": "ctrl+a",
    "command":
    {
        "action": "splitPane",
        "commandline": "cmd /c exit"
    }
}
```

and repeatedly spam Ctrl-A it used to lead to crashes. That doesn't
happen anymore, because some other PR must've fixed that.

Reverting #17450 fixes the issue found in #17578: Because the content
pointer didn't get reset to null anymore it meant that the root
pane retained the pointer after a split. After closing the split off
pane, it would assign the remaining one back to the root, which would
cause the still existing content pointer to be closed. That pointer
is potentially the same as the remaining pane and so no close events
would get received anymore.

Closes #17578

## Validation Steps Performed
* Add the above action and spam it ✅
* Start with an empty window, split pane, type `exit` in the new pane
  then type it in the original pane. It closes the window ✅

(cherry picked from commit 056af839940495989409d69e72e89311b1a7f209)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgSGCfE
Service-Version: 1.21
This commit is contained in:
Leonard Hecker 2024-08-21 14:44:42 +02:00 committed by Dustin L. Howett
parent 8c2c88e44f
commit 0cbe706f48
1 changed files with 3 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/cascadia/TerminalApp/Pane.cpp
View File

@ -1711,9 +1711,7 @@ void Pane::_SetupChildCloseHandlers()
IPaneContent Pane::_takePaneContent()
{
_closeRequestedRevoker.revoke();
// 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;
return std::move(_content);
}
// This method safely sets the content of the Pane. It'll ensure to revoke and
@ -1723,9 +1721,9 @@ 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 (_takePaneContent())
if (const auto c = _takePaneContent())
{
_content.Close();
c.Close();
}
if (content)