Skip to content

We play weird ownership games with SignalTextChanged, and we don't know why #13896

Open
@zadjii-msft

Description

@zadjii-msft

See context: #13876, MSFT: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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area-AccessibilityIssues related to accessibilityArea-CodeHealthIssues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.Area-TerminalControlIssues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.)Needs-Tag-FixDoesn't match tag requirementsPriority-3A description (P3)Product-TerminalThe new Windows Terminal.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions