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.

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

Don't crash graph checking on invalid syntax #16588

Merged
merged 9 commits into from
Jan 29, 2024

Conversation

0101
Copy link
Contributor

@0101 0101 commented Jan 25, 2024

Another case similar to #16575

Ends up throwing in VS with Transparent Compiler as you're typing.

@0101 0101 requested a review from a team as a code owner January 25, 2024 14:12
Copy link
Contributor

github-actions bot commented Jan 25, 2024

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@majocha
Copy link
Contributor

majocha commented Jan 25, 2024

Another surfacing exception:
image
Just after enabling it in options.

@0101
Copy link
Contributor Author

0101 commented Jan 25, 2024

Just after enabling it in options.

That must have to do with the fact that it needs a restart to actually start using it. But some of the options are read on every file check so there must be some mismatch. Should hopefully be easy to fix.

@0101
Copy link
Contributor Author

0101 commented Jan 25, 2024

The test failure here also looks suspicious, most likely a bug.

The active test run was aborted. Reason: Test host process crashed : Unhandled exception. System.ObjectDisposedException: The CancellationTokenSource has been disposed.
   at System.Threading.CancellationTokenSource.Cancel()
   at FSharp.Compiler.GraphChecking.GraphProcessing.raiseExn@233[a](CancellationTokenSource localCts, TaskCompletionSource`1 completionSignal, a item, Exception ex) in /home/vsts/work/1/s/src/Compiler/Driver/GraphChecking/GraphProcessing.fs:line 234
   at FSharp.Compiler.GraphChecking.GraphProcessing.queueNode@253-7.Invoke(FSharpChoice`2 res) in /home/vsts/work/1/s/src/Compiler/Driver/GraphChecking/GraphProcessing.fs:line 253
   at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvokeNoHijackCheck[a,b](AsyncActivation`1 ctxt, b result1, FSharpFunc`2 userCode) in /home/vsts/work/1/s/src/FSharp.Core/async.fs:line 528
   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in /home/vsts/work/1/s/src/FSharp.Core/async.fs:line 112
--- End of stack trace from previous location ---
   at Microsoft.FSharp.Control.AsyncPrimitives.Start@1173-1.Invoke(ExceptionDispatchInfo edi) in /home/vsts/work/1/s/src/FSharp.Core/async.fs:line 1173
   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in /home/vsts/work/1/s/src/FSharp.Core/async.fs:line 112
   at <StartupCode$FSharp-Core>.$Async.clo@193-15.Invoke(Object o) in /home/vsts/work/1/s/src/FSharp.Core/async.fs:line 195
   at System.Threading.QueueUserWorkItemCallback.Execute()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart()

@0101 0101 added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Jan 25, 2024
@0101 0101 enabled auto-merge (squash) January 26, 2024 10:50
Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

Sorry I crashed your merge but otherwise good stuff - thanks :)

@vzarytovskii vzarytovskii disabled auto-merge January 26, 2024 15:56
@vzarytovskii vzarytovskii enabled auto-merge (squash) January 26, 2024 15:57
@vzarytovskii vzarytovskii merged commit b3a7e29 into dotnet:main Jan 29, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants