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.

Sign up for GitHub

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

Jump to bottom

Conhost crash when using accessibility tools #3209

Closed
j4james opened this issue Oct 15, 2019 · 1 comment · Fixed by #3213
Closed

Conhost crash when using accessibility tools #3209

j4james opened this issue Oct 15, 2019 · 1 comment · Fixed by #3213
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase 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
Terminal-1910

Comments

@j4james
Copy link
Collaborator

j4james commented Oct 15, 2019

Environment

Windows build number: Version 10.0.18362.295
Conhost build: commit df26c67

Any other software? Inspect from the 10.0.18362.0 SDK

Steps to reproduce

  1. Start a conhost session.
  2. Start the Inspect utility.
  3. Click on the top level window in the UI Automation tree that matches the conhost session.

Expected behavior

You should see details about the window in the right hand pane.

Actual behavior

I get an access violation in WindowUiaProviderBase::get_ProviderOptions because pOptions has a value of 0x13, which isn't a valid pointer. The crash can also happen in other places depending on what accessibility call is triggered first.

I believe this is a regression thatwas introduced in commit cdfbf8f.

It's been a while since I've worked with COM, but I'm fairly certain you can't return an interface without a cast the way it is now being done in those QueryInterface methods. Otherwise the returned interface pointer won't be pointing to the correct vtable. And I suspect that's resulting in the wrong method being called by the code that is using the returned interface, hence the bogus parameter value (the parameter was likely intended for a completely different method).
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Oct 15, 2019
@DHowett-MSFT
Copy link
Contributor

Yeah ☹️ this'll be fixed by #3051, but we should get something in a bit quicker.

DHowett-MSFT pushed a commit that referenced this issue Oct 15, 2019
@carlos-zamora @DHowett
Switch all of the UIA providers to WRL::RuntimeClass
b0f0d69 
Fixes #3209.
References #3051.
This was referenced Oct 15, 2019
Closed
Merged
@ghost ghost added the In-PR This issue has a related PR label Oct 15, 2019
@DHowett-MSFT DHowett-MSFT added Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 17, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 17, 2019
@DHowett-MSFT DHowett-MSFT added this to the Terminal-1910 milestone Oct 17, 2019
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Oct 17, 2019
miniksa pushed a commit that referenced this issue Oct 17, 2019
@DHowett @carlos-zamora @miniksa
Switch all of the UIA providers to WRL::RuntimeClass (#3213)
4991b9f 
* Switch all of the UIA providers to WRL::RuntimeClass
Fixes #3209.
References #3051.

Co-authored-by: Carlos Zamora <cazamor@microsoft.com>
Co-authored-by: Dustin Howett <duhowett@microsoft.com>
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase 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.
Projects
None yet
Milestone
Terminal-1910
Development

Successfully merging a pull request may close this issue.

Switch all of the UIA providers to WRL::RuntimeClass microsoft/terminal
2 participants
@j4james @DHowett-MSFT