starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 05:35:20 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Crash when opening and closing a tab #5297

New Issue
Closed
opened 2026-01-31 00:09:57 +00:00 by claunia · 11 comments
claunia commented 2026-01-31 00:09:57 +00:00
Owner
Copy Link

Originally created by @timkoers on GitHub (Nov 29, 2019).

Environment

Windows 10 Home 18363.476
Windows Terminal (Preview) Store installation - v0.7.3291.0

When opening and closing a 3rd tab, when two tabs are already openend and a single tab is executing a command, the Terminal crashes completely.

I had a Windows PowerShell tab open as my first tab and a Ubuntu shell open as my second tab, opening and closing a new Windows PowerShell tab as the third one.

Originally created by @timkoers on GitHub (Nov 29, 2019). Environment ``` Windows 10 Home 18363.476 Windows Terminal (Preview) Store installation - v0.7.3291.0 ``` When opening and closing a 3rd tab, when two tabs are already openend and a single tab is executing a command, the Terminal crashes completely. I had a Windows PowerShell tab open as my first tab and a Ubuntu shell open as my second tab, opening and closing a new Windows PowerShell tab as the third one.
claunia commented 2026-01-31 00:09:58 +00:00
Author
Owner
Copy Link

@mkitzan commented on GitHub (Nov 30, 2019):

I found a very consistent way to crash WT when trying to replicate your issue. Have at least one tab open then in quick sequence use open your WSL terminal (using your keyboard shortcut) then enter the keyboard shortcut to close a tab. I have WSL mapped to Ctrl+Shift+1 and close tab on Ctrl+W. So by hitting Ctrl+Shift+1 then immediately Ctrl+W, WT crashes every time.
Edit: looks like during the creation of the Tab there's a null pointer de-reference in one of the auto generated WinRT functions (L839 of Windows.UI.Xaml.h for those with the dev build).

@mkitzan commented on GitHub (Nov 30, 2019): I found a very consistent way to crash WT when trying to replicate your issue. Have at least one tab open then in quick sequence use open your WSL terminal (using your keyboard shortcut) then enter the keyboard shortcut to close a tab. I have WSL mapped to Ctrl+Shift+1 and close tab on Ctrl+W. So by hitting Ctrl+Shift+1 then immediately Ctrl+W, WT crashes every time. Edit: looks like during the creation of the Tab there's a null pointer de-reference in one of the auto generated WinRT functions (L839 of Windows.UI.Xaml.h for those with the dev build).
claunia commented 2026-01-31 00:09:59 +00:00
Author
Owner
Copy Link

@mkitzan commented on GitHub (Nov 30, 2019):

Did a check of other issues. This issues sounds like a duplicate/instance of #2248.

@mkitzan commented on GitHub (Nov 30, 2019): Did a check of other issues. This issues sounds like a duplicate/instance of #2248.
claunia commented 2026-01-31 00:09:59 +00:00
Author
Owner
Copy Link

@DHowett-MSFT commented on GitHub (Dec 2, 2019):

In addition, there's at least one instance of us keeping a reference to a predeceased terminal control or tab object during a title changed event. We need to do a pass to make our async event handlers take weak references!

@DHowett-MSFT commented on GitHub (Dec 2, 2019): In addition, there's at least one instance of us keeping a reference to a predeceased terminal control or tab object during a title changed event. We need to do a pass to make our async event handlers take weak references!
claunia commented 2026-01-31 00:10:00 +00:00
Author
Owner
Copy Link

@mkitzan commented on GitHub (Dec 3, 2019):

@DHowett this crash instance is due to the Tab capturing itself as a raw pointer in the title change event handler and being de-referenced past its lifetime (as you suspected). In order for Tab to support WinRT weak_ref, we'll have to turn it into a WinRT object so it can be a valid template argument when calling get_weak or make_weak. I'll work on figuring out how to do that.
Bonus raw pointer capture in the same file. Once I figure out the pattern for fixing these, I can do a pass fixing raw pointer captures.

@mkitzan commented on GitHub (Dec 3, 2019): @DHowett this crash instance is due to the `Tab` [capturing itself as a raw pointer](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalApp/Tab.cpp#L299) in the title change event handler and being de-referenced past its lifetime (as you suspected). In order for `Tab` to support WinRT `weak_ref`, we'll have to turn it into a WinRT object so it can be a [valid template argument](https://docs.microsoft.com/en-us/uwp/cpp-ref-for-winrt/weak-ref#template-parameters) when calling `get_weak` or `make_weak`. I'll work on figuring out how to do that. Bonus [raw pointer capture](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalApp/Tab.cpp#L319) in the same file. Once I figure out the pattern for fixing these, I can do a pass fixing raw pointer captures.
claunia commented 2026-01-31 00:10:00 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Dec 3, 2019):

@mkitzan it might be easier to change Tab to enable_shared_from_this, and then get a weak pointer to itself. Making the Tab a proper WinRT type is something we'd definitely want to do in the future, but we might want to get this particular crash solved faster than that :P

@zadjii-msft commented on GitHub (Dec 3, 2019): @mkitzan it might be easier to change `Tab` to [`enable_shared_from_this`](https://en.cppreference.com/w/cpp/memory/enable_shared_from_this), and then get a weak pointer to itself. Making the `Tab` a proper WinRT type is something we'd definitely want to do in the future, but we might want to get this particular crash solved faster than that :P
claunia commented 2026-01-31 00:10:01 +00:00
Author
Owner
Copy Link

@mkitzan commented on GitHub (Dec 3, 2019):

Sounds like a good plan for the time being. I'll add a TODO in the source to eventually make a Tab WinRT type.

@mkitzan commented on GitHub (Dec 3, 2019): Sounds like a good plan for the time being. I'll add a TODO in the source to eventually make a `Tab` WinRT type.
claunia commented 2026-01-31 00:10:01 +00:00
Author
Owner
Copy Link

@mkitzan commented on GitHub (Dec 4, 2019):

@zadjii-msft currently the event handlers are bound to the TermControl and root Pane on construction of Tab. The lambdas can't correctly capture the std::weak_ptr<Tab> because they require the object spawning the std::weak_ptr<Tab> to be of type std::shared_ptr<Tab> (instead of plain Tab). So, to get this to work I'm thinking of adding a weird Tab function member to provide a Tab object with a std::weak_ptr<Tab> to itself before binding the events. Any thoughts on this, because it seems a little 🙃
If there isn't a clean work around, it may make sense to just bite the bullet and make Tab a WinRT type (especially considering we'll probably roll-back this work when it becomes an WinRT type).

@mkitzan commented on GitHub (Dec 4, 2019): @zadjii-msft currently the event handlers are bound to the `TermControl` and root `Pane` on construction of `Tab`. The lambdas can't correctly capture the `std::weak_ptr<Tab>` because they require the object spawning the `std::weak_ptr<Tab>` to be of type `std::shared_ptr<Tab>` (instead of plain `Tab`). So, to get this to work I'm thinking of adding a weird `Tab` function member to provide a `Tab` object with a `std::weak_ptr<Tab>` to itself before binding the events. Any thoughts on this, because it seems a little 🙃 If there isn't a clean work around, it may make sense to just bite the bullet and make `Tab` a WinRT type (especially considering we'll probably roll-back this work when it becomes an WinRT type).
claunia commented 2026-01-31 00:10:01 +00:00
Author
Owner
Copy Link

@DHowett-MSFT commented on GitHub (Dec 4, 2019):

If we make Tab inherit from enable_shared_from_this<Tab>, it'll gain a method shared_from_this() that will return a std::shared_ptr<Tab> that you can then weaken 😄

@DHowett-MSFT commented on GitHub (Dec 4, 2019): If we make Tab inherit from `enable_shared_from_this<Tab>`, it'll gain a method `shared_from_this()` that will return a `std::shared_ptr<Tab>` that you can then weaken :smile:
claunia commented 2026-01-31 00:10:02 +00:00
Author
Owner
Copy Link

@mkitzan commented on GitHub (Dec 4, 2019):

Thanks @DHowett-MSFT, got too focused on the weak_ptr side of things! Using shared_from_this() still requires a function member to initialize the event binding, because calling shared_from_this() inside the constructor throws bad_weak_ptr exception. Luckily, calling the binding function could fit well in TerminalPage::_RegisterTerminalEvents or just after Tab construction. This fixes the issue, no more crashes when rapidly opening-closing tabs!

@mkitzan commented on GitHub (Dec 4, 2019): Thanks @DHowett-MSFT, got too focused on the `weak_ptr` side of things! Using `shared_from_this()` still requires a function member to initialize the event binding, because calling `shared_from_this()` inside the constructor throws `bad_weak_ptr` exception. Luckily, calling the binding function could fit well in [`TerminalPage::_RegisterTerminalEvents`](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalApp/TerminalPage.cpp#L756) or just after [`Tab` construction](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalApp/TerminalPage.cpp#L435). This fixes the issue, no more crashes when rapidly opening-closing tabs!
claunia commented 2026-01-31 00:10:02 +00:00
Author
Owner
Copy Link

@ghost commented on GitHub (Dec 12, 2019):

:tada:This issue was addressed in #3835, which has now been successfully released as Windows Terminal Preview v0.7.3451.0.🎉

Handy links:

@ghost commented on GitHub (Dec 12, 2019): :tada:This issue was addressed in #3835, which has now been successfully released as `Windows Terminal Preview v0.7.3451.0`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v0.7.3451.0) * [Store Download](https://www.microsoft.com/store/apps/9n0dx20hk701?cid=storebadge&ocid=badge)
claunia commented 2026-01-31 00:10:03 +00:00
Author
Owner
Copy Link

@ghost commented on GitHub (Jan 14, 2020):

:tada:This issue was addressed in #3835, which has now been successfully released as Windows Terminal Preview v0.8.10091.0.🎉

Handy links:

@ghost commented on GitHub (Jan 14, 2020): :tada:This issue was addressed in #3835, which has now been successfully released as `Windows Terminal Preview v0.8.10091.0`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v0.8.10091.0) * [Store Download](https://www.microsoft.com/store/apps/9n0dx20hk701?cid=storebadge&ocid=badge)
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Help Wanted Issue-Bug Needs-Tag-Fix Product-Terminal Resolution-Fix-Committed Severity-Crash
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#5297
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?