#582 VS 2013 Crash NullReferenceException

[mousetraps]

• it was sometimes possible for one of the children in moduleTree to be
  null. Check for null before mod.Name string comparison.

fix #582

[mousetraps]

I couldn't actually reproduce this issue, but as described in #582 (comment), this is the only way that a NRE could be thrown with the provided call stack.

Still haven't conclusively narrowed down how the tree could be null, but I believe it may be happening here:
https://github.com/Microsoft/nodejstools/blob/master/Nodejs/Product/Analysis/Analysis/ProjectEntry.cs#L106

[mousetraps]

@DinoV Any chance you could review this? It's not entirely clear to me in what scenarios mod might be null here

[billti]

This was the same defensive code I was considering adding, but still couldn't figure out why this would ever occur :-/

Barring any deeper understanding of what is causing this, looks like the right change to make to me 👍

[DinoV]

I suspect it is actually a race condition, especially given that it's reported to be intermittent. ModuleTree.Children is a non-thread safe dictionary, and AddChildVisibilitiesExcludingNodeModules is accessing it w/o holding the ModuleTable lock. _table.RequireModule will acquire the lock, do the lookup, and release it. Then we access the children in a non-thread safe manner, so we can come back with a corrupted dictionary that has null values (or get other strange behavior).

It probably makes sense to have a ModuleTable.GetChildrenExcludingNodeModules which does the RequireModule + GetChildrenExcludingNodeModules with the lock held the entire time.