Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix infinite recursion in TSF #18248

Merged
merged 3 commits into from
Nov 26, 2024
Merged

Fix infinite recursion in TSF #18248

merged 3 commits into from
Nov 26, 2024

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Nov 25, 2024

The current FindWindowOfActiveTSF implementation can
result in infinite recursion which we must guard again.
This change is not tested as I don't know how to trigger
the issue to begin with (a missing CoreInput thread).

@lhecker lhecker added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Priority-1 A description (P1) labels Nov 25, 2024
// This can then in infinite recursion because we're calling
// GetWnd() below, which may call GetHwnd(), which may call
// FindWindowOfActiveTSF(), and so on.
// By temporarily clearing the _provider we fix that flaw.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the determination to bail out made based on _provider?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throughout the file. All places that use _provider are already doing a null check.

@DHowett DHowett enabled auto-merge (squash) November 25, 2024 22:51
@DHowett DHowett merged commit 9243104 into main Nov 26, 2024
20 checks passed
@DHowett DHowett deleted the dev/lhecker/tsf-recursion branch November 26, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal.
Projects
Status: To Cherry Pick
Status: To Cherry Pick
Development

Successfully merging this pull request may close these issues.

3 participants