removed ImmutableArray from json.net usage. and moved to IList

[heejaechang]

this is port of #21395 to dev15.3.x branch.

Customer scenario

VS crashes at random point when C#/VB proects are loaded to VS.

Bugs this fixes:

https://devdiv.visualstudio.com/DevDiv/_workitems?id=473979&fullScreen=false&_a=edit

Workarounds, if any

Make sure VS uses newtonsoft.json shipped with VS not one in the GAC or one in some VS extensions by deleting those.

Risk

low. There should be no behavior or noticeable perf changes. it just changes collection type we use to marshal collections between 2 processes.

Performance impact

there could be some allocations differences due to creating different kinds of collections (List vs ImmutableArray) but not significant.

Is this a regression from a previous update?

Kind of yes. in RTM, we used old version of newtonsoft.json to build, so didn't try to use ImmutableArray, after RTM, we moved all dependencies to current (what VS uses) which support ImmutableArray, so we changed all code to use ImmutableArray.

Root cause analysis:

This is general problem for any dll that are not specific to VS but VS depends on. we just encountered it since newstonsoft.json is such a popular one. and some other software GACed the dll that happened to be same version as VS uses but target different framework.

How was the bug found?

Watson. feedbacks.

[heejaechang]

@Pilchie is this right branch?

[heejaechang]

@dotnet/roslyn-analysis @dotnet/roslyn-ide can you take a look? we want to bring this to escrow.

this is port of #21395 to dev15.3.x branch.

[Pilchie]

Yes, this is the right branch.

Can we work on extra validation for this please? Ensure we have a machine set up with the problematic dll in the GAC, verify the crash repros without the fix, doesn't repro with the fix, as well as testing the features affected by these changes.

@richaverma1 can help coordinate some of the testing here.

[Pilchie]

These changes look right to me, but I'm wondering what we can do to ensure it's the entire set?

[CyrusNajmabadi]

@heejaechang Did you just do a cherry-pick here? Are there any extra changes you had to make manually? If so, can you point them out?

[heejaechang]

I didn't do cherry-pick since code is quite different. (master uses new StreamRpc version that uses InvokeWithCancellation and dev15.3.x uses old InvokeAsync which have cancellation at the session)

I went each file and manually re-did the work.

[CyrusNajmabadi]

ok. i can review later tonight then!

[heejaechang]

@Pilchie sure, let me do some verification today. can I ask vendor to do some testing as well?

[heejaechang]

done testing. found some issues and pushed more changes. @Pilchie can I get some help from vendor on doing more testing on these?

[brettfo]

Merging at the request of @Pilchie.