[PR #13876] [MERGED] An attempted fix for the SignalTextChanged crash #29795

Open
opened 2026-01-31 09:36:56 +00:00 by claunia · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/13876
Author: @zadjii-msft
Created: 8/29/2022
Status: Merged
Merged: 9/2/2022
Merged by: @undefined

Base: mainHead: dev/migrie/b/13357-attempt-0


📝 Commits (3)

  • bd466ab An attempted fix for the SignalTextChanged crash
  • ae172c4 Apply suggestions from code review
  • db00bf6 more cleanup

📊 Changes

3 files changed (+51 additions, -27 deletions)

View changed files

📝 src/cascadia/TerminalControl/TermControl.cpp (+1 -1)
📝 src/cascadia/TerminalControl/TermControlAutomationPeer.cpp (+48 -24)
📝 src/cascadia/TerminalControl/TermControlAutomationPeer.h (+2 -2)

📄 Description

This is conjecture - I was totally unable to repro the original crash here.
Based on the stacks in MSFT:39994969, it looks like we try to fire off a
RaiseAutomationEvent, which calls through UIA core, eventually to the point of
calling ComPtr<WUX::Automation::Peers::IAutomationPeer>::{dtor}. I'm guessing
based on the stacks that the TermControl has already been released and cleaned
up. However, the lambda in the RunAsync calls here only takes a ref to the
TCAP. The TCAP has an outstanding reference (maybe on the other side of the UIA
fence), and gets successfully resolved as strong, but when calling to
RaiseAutomationEvent, the owner we passed in is gonezo.

This explicitly passes a weak_ref to TermControlAutomationPeer, rather than
a raw ptr, so we can actually check if the control is still alive before we
dereference it. If it is, great, we've got a strong ref to it now and it won't
get torn down.

Again, this is hearsay. Without a repro, the only way we can confirm this is
gone is by just hoping the crashes go away. 🤞

  • Might close #13357 (we'll reopen if it doesn't?)
  • narrator still works

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/microsoft/terminal/pull/13876 **Author:** [@zadjii-msft](https://github.com/zadjii-msft) **Created:** 8/29/2022 **Status:** ✅ Merged **Merged:** 9/2/2022 **Merged by:** [@undefined](undefined) **Base:** `main` ← **Head:** `dev/migrie/b/13357-attempt-0` --- ### 📝 Commits (3) - [`bd466ab`](https://github.com/microsoft/terminal/commit/bd466ab156c35af4f5642ab1e0f1578f2597fbb1) An attempted fix for the `SignalTextChanged` crash - [`ae172c4`](https://github.com/microsoft/terminal/commit/ae172c4b664a81b488726b805053d22534c9b4ef) Apply suggestions from code review - [`db00bf6`](https://github.com/microsoft/terminal/commit/db00bf6d3b5848dd8a4dc509d124761edab06cdf) more cleanup ### 📊 Changes **3 files changed** (+51 additions, -27 deletions) <details> <summary>View changed files</summary> 📝 `src/cascadia/TerminalControl/TermControl.cpp` (+1 -1) 📝 `src/cascadia/TerminalControl/TermControlAutomationPeer.cpp` (+48 -24) 📝 `src/cascadia/TerminalControl/TermControlAutomationPeer.h` (+2 -2) </details> ### 📄 Description This is conjecture - I was totally unable to repro the original crash here. Based on the stacks in MSFT:39994969, it looks like we try to fire off a `RaiseAutomationEvent`, which calls through UIA core, eventually to the point of calling `ComPtr<WUX::Automation::Peers::IAutomationPeer>::{dtor}`. I'm guessing based on the stacks that the TermControl has already been released and cleaned up. However, the lambda in the `RunAsync` calls here only takes a ref to the TCAP. The TCAP has an outstanding reference (maybe on the other side of the UIA fence), and gets successfully resolved as strong, but when calling to `RaiseAutomationEvent`, the `owner` we passed in is gonezo. This explicitly passes a `weak_ref` to `TermControlAutomationPeer`, rather than a raw ptr, so we can actually check if the control is still alive before _we_ dereference it. If it is, great, we've got a strong ref to it now and it won't get torn down. Again, this is hearsay. Without a repro, the only way we can confirm this is gone is by just hoping the crashes go away. 🤞 * Might close #13357 (we'll reopen if it doesn't?) * narrator still works --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-31 09:36:56 +00:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#29795