Access Violation in TerminalControl!winrt::impl::consume_Windows_UI_Xaml_Controls_Primitives_IRangeBase<winrt::Windows::UI::Xaml::Controls::Primitives::ScrollBar>::Maximum #2947
Labels
Area-TerminalControl
Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.)
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Priority-1
A description (P1)
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Severity-Crash
Crashes are real bad news.
Milestone
Comments
ghost
added
Needs-Triage
|
Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report! |
ghost
added
Resolution-Duplicate
5 tasks
Closed
|
Since we can fix this one separately, I'm going to reopen it. |
DHowett-MSFT
added
Area-TerminalControl
|
There doesn't seem to be a pull-request for this? #3256 specifically excluded this as being fixed. |
|
@TBBle good catch. Thanks! |
|
@zadjii-msft I'm not sure this is a duplicate of #2248 since the call stack indicates this exception is from |
|
After fixing the lambdas in |
ghost
added
Resolution-Fix-Committed
ghost
pushed a commit
that referenced
this issue
Dec 13, 2019
<!-- 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.
|
🎉This issue was addressed in #3908, which has now been successfully released as Handy links: |
donno2048
added a commit
to donno2048/terminal
that referenced
this issue
Sep 28, 2020
<!-- 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](microsoft/terminal#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.
This issue was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Environment
Steps to reproduce
dir c:\ -recExpected behavior
The tab closes.
Actual behavior
The terminal crashes with an Access Violation due to a null pointer dereference in
TerminalControl!winrt::impl::consume_Windows_UI_Xaml_Controls_Primitives_IRangeBase<winrt::Windows::UI::Xaml::Controls::Primitives::ScrollBar>::MaximumNotes
I can reproduce this consistently. Let me know if you would like a memory dump.
Here is a link to the feedback issue I opened:
https://aka.ms/AA65up1
Call stack
The text was updated successfully, but these errors were encountered: