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

NULL_CLASS_PTR_READ from PublicTerminalCore.dll!TerminalIsSelectionActive #14461

Closed
javierdlg opened this issue Nov 29, 2022 · 4 comments · Fixed by #14678
Closed

NULL_CLASS_PTR_READ from PublicTerminalCore.dll!TerminalIsSelectionActive #14461

javierdlg opened this issue Nov 29, 2022 · 4 comments · Fixed by #14678
Assignees
@carlos-zamora
Labels
Area-WPFControl Things related to the WPF version of the TermControl 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
Terminal v1.17

Comments

@javierdlg
Copy link
Member

Windows Terminal version

1.15.2210.6001

Windows build number

10.0.19044.2251

Other Software

Visual Studio 2022 v.17.5.33103.201

Steps to reproduce

Currently unknown, but the dumps show the user selecting some text and Visual Studio crashing due to an AccessViolationException.

You can find more information in the Watson cabs:
https://watsonportal.microsoft.com/Failure?FailureSearchText=32503620-0791-0795-1eff-dc016b5e23e9&MaxRows=100

Internal bug ticket:
Bug 1676887: [Watson] crash64: NULL_CLASS_PTR_READ_c0000005_PublicTerminalCore.dll!TerminalIsSelectionActive

Version of VS showing this issue:
https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/1639645
(Go to Build and Install tab for download link)

Expected Behavior

No response

Actual Behavior

Visual Studio crashes when text is selected
@javierdlg javierdlg added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Nov 29, 2022
@zadjii-msft zadjii-msft added Severity-Crash Crashes are real bad news. Product-Terminal The new Windows Terminal. Priority-1 A description (P1) Area-WPFControl Things related to the WPF version of the TermControl and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Nov 30, 2022
@zadjii-msft zadjii-msft added this to the Terminal v1.17 milestone Nov 30, 2022
@zadjii-msft
Copy link
Member

I can't get at the dumps on this machine, but the blamed stack frame is in

terminal/src/cascadia/PublicTerminalCore/HwndTerminal.cpp

Lines 609 to 614 in f2eed92

bool _stdcall TerminalIsSelectionActive(void* terminal)
{
const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
const auto selectionActive = publicTerminal->_terminal->IsSelectionActive();
return selectionActive;
}

There's no guards there. There probably should be, but also, in what scenario is there ever a null publicTerminal, or a null publicTerminal->_terminal? I'm pretty sure there shouldn't be...

@zadjii-msft
Copy link
Member

image

0b 00000077`7a8fd4c0 00007ffb`8c358530     ntdll!KiUserExceptionDispatch+0x2e [minkernel\ntos\rtl\amd64\trampoln.asm @ 755] 
0c 00000077`7a8fdc68 00007ffb`a75ce837     PublicTerminalCore!TerminalIsSelectionActive [C:\a\_work\1\s\src\cascadia\PublicTerminalCore\HwndTerminal.cpp @ 610] 
0d 00000077`7a8fdc70 00007ffb`a75ce774     0x00007ffb`a75ce837
0e 00000077`7a8fdd20 00007ffb`a75ce677     0x00007ffb`a75ce774
0f 00000077`7a8fdd50 00007ffb`8a86e417     0x00007ffb`a75ce677
...

well, that's... not what I'd expect

@carlos-zamora carlos-zamora self-assigned this Jan 11, 2023
@carlos-zamora carlos-zamora mentioned this issue Jan 13, 2023
Merged
@ghost ghost added the In-PR This issue has a related PR label Jan 13, 2023
@ghost ghost closed this as completed in #14678 Jan 13, 2023
@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 Jan 13, 2023
ghost pushed a commit that referenced this issue Jan 13, 2023
@carlos-zamora
[WPF] Add TermCore null checks to HwndTerminal (#14678)
dc6d82e 
#14461 is caused by a null pointer exception in `TerminalIsSelectionActive()`. Since this reveals that it is possible to enter a state where `_terminal` is null, I've gone ahead and added null-checks throughout the code to make it more stable.

Closes #14461

## Validation Steps Performed
Ran locally. Still works.
DHowett pushed a commit that referenced this issue Jan 20, 2023
@carlos-zamora @DHowett
[WPF] Add TermCore null checks to HwndTerminal (#14678)
bb67c1b 
Closes #14461

Ran locally. Still works.

(cherry picked from commit dc6d82e)
Service-Card-Id: 87602014
Service-Version: 1.16
@ghost
Copy link

ghost commented Jan 24, 2023

🎉This issue was addressed in #14678, which has now been successfully released as Windows Terminal v1.16.1023 (10231 and 10232).:tada:

Handy links:

@ghost
Copy link

ghost commented Jan 24, 2023

🎉This issue was addressed in #14678, which has now been successfully released as Windows Terminal Preview v1.17.1023.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@carlos-zamora carlos-zamora

Labels
Area-WPFControl Things related to the WPF version of the TermControl 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.
Projects
None yet
Milestone
Terminal v1.17
Development

Successfully merging a pull request may close this issue.

[WPF] Add TermCore null checks to HwndTerminal microsoft/terminal
3 participants
@javierdlg @carlos-zamora @zadjii-msft