microsoft / microsoft/terminal
We play weird ownership games with SignalTextChanged, and we don't know why
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 105k
- Forks
- 9.6k
- Avg merge
- 3d 17h
- Merged PRs (30d)
- 29
Description
See context: #13876, [MSFT:39994969](https://task.ms/39994969)
See discussion:
> I'm not really in favor of this PR. There shouldn't be situation to begin with were NVDA calls us and the existence of `TermControl` is the make/break of whether WT crashes... As such I'd prefer fixing whatever UIA APIs we have that might crash WT, including any assumptions we have that WinUI won't suddenly call `AppHost::~AppHost` despite being 50 calls deep in the stack.
> > Should we file a workitem to fix this more soundly? Should we take this as a "stopgap" until we can fix it more soundly?
>
> Maybe I was too terse. 😄 I'm in favor of merging this, just not in favor of this PR because it's a stopgap. I think we should just continue to work on cleaning up our pointer safety and data sharing behavior. Maybe even do it consciously at some point instead of doing it on a case by case basis. IMO we should remove any use of `this` (including implicit use) if it can be avoided and find and if possible remove implicit recursion like this one were class X calls Y which calls back into X (e.g. introduce class Z which is used by both X and Y, removing the need for recursion).
> > Maybe even do it consciously at some point instead of doing it on a case by case basis.
>
> YES
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with issue #13876 and the linked MSFT work item, then trace SignalTextChanged, TermControl, and the UIA calls discussed here. The work is done when the ownership, pointer-safety, and data-sharing behavior no longer depend on the described stopgap or implicit recursive calls.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- cli, operating-systems
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 20/100