-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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 a race condition in UpdatePatternLocations #13859
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
@msftbot merge this in 5 minutes |
Hello @carlos-zamora! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
Since locking the console can take a non-trivial amount of time, the main thread might have already released the ControlCore instance, while the `UpdatePatternLocations` background thread is holding the last active reference. If the function call goes out of scope, we destroy the instance, which might not be safe, considering its members are usually only used by the main thread. This commit fixes the issue by only holding a reference of the `Terminal`. ## Validation Steps Performed * Patterns are recognized ✅ (cherry picked from commit 12122b2) Service-Card-Id: 85321064 Service-Version: 1.15
🎉 Handy links: |
🎉 Handy links: |
Since locking the console can take a non-trivial amount of time,
the main thread might have already released the ControlCore instance, while the
UpdatePatternLocations
background thread is holding the last active reference.If the function call goes out of scope, we destroy the instance, which might
not be safe, considering its members are usually only used by the main thread.
This commit fixes the issue by only holding a reference of the
Terminal
.Validation Steps Performed