-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fixed self capture in TermControl (#3908)
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request An asynchronous event handler capturing raw `this` in `TermControl` was causing an exception to be thrown when a scroll update event occurred after closing the active tab. This PR replaces all non-auto_revoke lambda captures in `TermControl` to capture (and validate) a `winrt::weak_ref` instead of using raw `this`. <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes #2947 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Requires documentation to be updated * [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #2947 <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments `TermControl` is already a WinRT type so no changes were required to enable the `winrt::weak_ref` functionality. There was only one strange change I had to make. In the destructor's helper function `Close`, I had to remove two calls to [`Stop`](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.dispatchertimer.stop#Windows_UI_Xaml_DispatcherTimer_Stop) which were throwing under [some circumstances](#2947 (comment)). Fortunately, these calls don't appear to be critical, but definitely a spot to look into when reviewing this PR. Beyond scrolling, any anomalous crash related to the following functionality while closing a tab or WT may be fixed by this PR: - Settings updating - Changing background color <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed Before these changes I was able to consistently repro the issue in #2947. Now, I can no longer repro the issue.
- Loading branch information
Showing
2 changed files
with
89 additions
and
81 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters