Skip to content

Conversation

@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented May 3, 2016

#917

Bug
If the Repl window is used before the main NTVS package has been loaded, NodeJsPacakge.Instance will be null. This causes a null deref problem in some call paths (see #917)

Fix
Add a few null checks to these call paths.

closes #917

… yet

**Bug**
If the Repl window is used before the main NTVS package has been loaded, `NodeJsPacakge.Instance` will be null. This causes a null deref problem

**Fix**
Add a few null checks.

closes microsoft#917
@mjbvz
Copy link
Contributor Author

mjbvz commented May 3, 2016

Actually, the change should not hurt anything, but it also does not solve a normal use case bug.

During development, I was messing around with the Repl window and this ended up persisting some bad data about the Repl Window to the registry under the ActiveRepls key. This seemed to have messed up loading, causing this bug to be hit. In normal user flows, I think that Node.Instance should always be set at these points in the code.

public static IWpfTextView GetActiveTextView(System.IServiceProvider serviceProvider) {
if (serviceProvider == null) {
return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

for both null checks, it's okay to fail gracefully, but let's go ahead and also add a debug assert so we know something's up when the issue occurs.

@mousetraps
Copy link
Contributor

minor comment and 👍 after that

@mjbvz mjbvz merged commit 21d0243 into microsoft:master May 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Null Dereference when Using Interactive Window Before Main NodeJs Package is loaded

3 participants