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

Prevent crash due to NullReferenceException in interactive window component #4589

Merged
merged 3 commits into from
Aug 2, 2018
Merged

Prevent crash due to NullReferenceException in interactive window component #4589

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9067eb7
Prevent crash due to NullReferenceException in interactive window com…
Jul 24, 2018
f100729
Catch exception only when shutting down, which is when the interactiv…
Jul 26, 2018
50a4ea6
Extract shutdown check into separate method.
Jul 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions ...Product/PythonTools/PythonTools/Repl/PythonInteractiveEvaluator.CommandProcessorThread.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,21 @@ private void ProcessExited(object sender, EventArgs e) {
private void StdErrReceived(object sender, DataReceivedEventArgs e) {
if (e.Data != null) {
if (!AppendPreConnectionOutput(e)) {
_eval.WriteError(FixNewLines(e.Data));
try {
_eval.WriteError(FixNewLines(e.Data));
} catch (Exception ex) when (!ex.IsCriticalException()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what causes NRE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the stack and links I've put in #4513

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, can we check Dispatcher.HasShutdownStarted in when condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How? With Dispatcher.CurrentDispatcher.HasShutdownStarted? Docs say that never returns null, that one is created if there isn't one. Will it have the same values as the dispatcher object in the interactive window:

private Dispatcher Dispatcher => ((FrameworkElement)_uiOnly.TextView).Dispatcher; // Always safe to access the dispatcher.

Copy link
Contributor

@AlexanderSher AlexanderSher Jul 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Always safe to access the dispatcher.

Well, that's actually not true: https://referencesource.microsoft.com/#WindowsBase/Base/System/Windows/Freezable.cs,887

We should be able to use _eval.CurrentWindow.TextView.VisualElement.Dispatcher == null

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just saw the exception while I was debugging, I was able verify that _eval.CurrentWindow.TextView.VisualElement.Dispatcher.HasShutdownStarted==true (and _eval.CurrentWindow.TextView.VisualElement.Dispatcher.hasShutdownFinished==true)

so I'll catch if shutdown started or dispatcher is null.

}
}
}
}

private void StdOutReceived(object sender, DataReceivedEventArgs e) {
if (e.Data != null) {
if (!AppendPreConnectionOutput(e)) {
_eval.WriteOutput(FixNewLines(e.Data));
try {
_eval.WriteOutput(FixNewLines(e.Data));
} catch (Exception ex) when (!ex.IsCriticalException()) {
}
}
}
}
Expand Down