Skip to content

Conversation

@brettfo
Copy link
Member

@brettfo brettfo commented Oct 21, 2017

No description provided.

@brettfo brettfo requested a review from KevinRansom October 21, 2017 02:41
@brettfo brettfo merged commit 37fc75d into dotnet:vs2017-rtm Oct 21, 2017
@brettfo brettfo deleted the source-file-count branch October 21, 2017 04:05
@brettfo
Copy link
Member Author

brettfo commented Oct 21, 2017

Before this change, the bug encountered was the following:

  1. Install the latest F# bits.
  2. In VS create a new F# .NET Core Console Application.
  3. VS could crash.

The crash was caused by a NullReferenceException here; the blockStructure returned 5 lines above was null. The GetBlockStructureAsync() method that was returning null is here. Inside the asyncMaybe block the call to checker.ParseDocument eventually called FSharpParsingOptions.LastFileName, but the SourceFiles property was empty causing the call to Array.last to throw. That exception was handled by RoslynHelpers.StartAsyncAsTask and this line is what created the null value that was eventually returned back. That null value shouldn't be returned and issue #3791 has been filed to improve the exception continuation.

To fix the issue of checker.ParseDocument being called when no documents are present is to only handle project updated events when there are actually source files present (this PR). The specific issue that led to this race condition was that after a new project has been created it's possible that the block structure tagger could be invoked before CPS has finished notifying the F# project system of the command line options, which means that the F# project system doesn't know about any files yet.

Tagging @KevinRansom to add his analysis as well, either as additional comments below or by directly editing this comment.

KevinRansom pushed a commit that referenced this pull request Oct 26, 2017
* don't update project info if the source file collection is empty (#3792)

* install templates VSIX to a unique directory (#3804)

* P2p references (#3777)

* P2p references

* Fix test

* test fix

* go faster stripes

* new project works better

* Re-add debug assert for sourcefiles

* Parameterise rc location (#3744)

* Fix issues
KevinRansom added a commit that referenced this pull request Oct 27, 2017
* Merge dev15.5 to dev15.6 (#3825)

* don't update project info if the source file collection is empty (#3792)

* install templates VSIX to a unique directory (#3804)

* P2p references (#3777)

* P2p references

* Fix test

* test fix

* go faster stripes

* new project works better

* Re-add debug assert for sourcefiles

* Parameterise rc location (#3744)

* Fix issues

* Merge master to dev15.6 (#3826)

* Put nupkgs into artifacts (#3806)

* Make FCS build work on Jenkins (#3788)
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
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.

1 participant