fix for race condition in FileIndex.fileIndexOfFile

[Martin521]

Description

Here is a fix for another ancient bug that I found while working on "scoped nowarn".
It took a while to find the culprit.
There is a race condition in FileIndex.fileIndexOfFile that can lead to different indices for the same file.
This PR contains the fix and a regression test.

[github-actions]

❗ Release notes required

---

✅ Found changes and release notes in following paths:

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/9.0.200.md

[Martin521]

Can somebody add the "No release notes" label? Thanks!

[psfinaki]

Actually you can probably add them - it's a bug fix, it's notable from the product perspective :)

(as opposed to refactoring or adding tests - they could be very useful but don't compose a new "release" really).

[Martin521]

That makes sense. I have added them.

BTW the change is small, hopefully easy to review.

[psfinaki]

Thanks for the contribution, Martin. The change is indeed small, but anything that touches critical parts of the compiler needs a focused review :)

We're a bit busy with .NET9 handling and some internal stuff right now, we hope to get to the PRs soon - thanks for your patience!

[Martin521]

Some remarks that might help in reviewing this PR:

• The issue is for real
  • I observed, in a "sometimes" failing "scoped nowarn" test, that FileToIndex sometimes returns different file indices for the same file name.
  • The test added in this PR reproduces this for the current compiler.
• The only thing that the fix does is to lock the whole read-check-write operation rather than just the write operations. I think it should be clear that the read-check-write must be an atomic operation.
• FileToIndex is called just once per compiled file (plus once per #line directive). So, there should be no performance impact. If there is still concern, we can add an unlocked read-check before the locked operations, so that the lock is entered only for new files. But I don't think it is worth it.

[psfinaki]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Martin521]

ILVerify succeeds locally for this same commit

[psfinaki]

I think I have something similar happening in my PR, it's very annoying indeed.

[vzarytovskii]

ILVerify succeeds locally for this same commit

It is indeed failing in this branch locally for me:

> git log -1
commit d4a64bd1a4ba1f21741ce922f8562c8a48b09521 (HEAD -> fileindex-race, Martin521/fileindex-race)
Merge: f1abc28b4 9ab4e3935
Author: Petr <psfinaki@users.noreply.github.com>
Date:   Mon Dec 9 15:42:57 2024 +0100

    Merge branch 'main' into fileindex-race
    
> pwsh tests/ILVerify/ilverify.ps1
pwsh tests/ILVerify/ilverify.ps1
Checking whether running on Windows: False
Repository path: /Users/u/code/fsharp4
...


InputObject
-----------                                                                                                                                                                                                                                                                                                                                                             
[IL]: Error [StackUnexpected]: : FSharp.Compiler.EditorServices.ParsedInput+visitor@1423-11::VisitExpr([FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<FSharp.Compiler.Syntax.SyntaxNode>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<FSharp.Compiler.Syntax.SynExpr,Microsoft.FSharp.Core.FSharpOption`1<FSharp.Compiler.EditorServices.CompletionContext>…
[IL]: Error [StackUnexpected]: : FSharp.Compiler.EditorServices.ParsedInput+visitor@1423-11::VisitExpr([FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<FSharp.Compiler.Syntax.SyntaxNode>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<FSharp.Compiler.Syntax.SynExpr,Microsoft.FSharp.Core.FSharpOption`1<FSharp.Compiler.EditorServices.CompletionContext>…

ILverify output matches baseline.
ILverify failed.

> git diff
diff --git a/tests/ILVerify/ilverify_FSharp.Compiler.Service_Release_netstandard2.0.bsl b/tests/ILVerify/ilverify_FSharp.Compiler.Service_Release_netstandard2.0.bsl
index cd2141db4..9de22b5cd 100644
--- a/tests/ILVerify/ilverify_FSharp.Compiler.Service_Release_netstandard2.0.bsl
+++ b/tests/ILVerify/ilverify_FSharp.Compiler.Service_Release_netstandard2.0.bsl
@@ -33,7 +33,7 @@
 [IL]: Error [StackUnexpected]: : <StartupCode$FSharp-Compiler-Service>.$FSharpCheckerResults+GetReferenceResolutionStructuredToolTipText@2205::Invoke([FSharp.Core]Microsoft.FSharp.Core.Unit)][offset 0x00000076][found Char] Unexpected type on the stack.
 [IL]: Error [StackUnexpected]: : FSharp.Compiler.EditorServices.AssemblyContent+traverseMemberFunctionAndValues@176::Invoke([FSharp.Compiler.Service]FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue)][offset 0x0000002B][found Char] Unexpected type on the stack.
 [IL]: Error [StackUnexpected]: : FSharp.Compiler.EditorServices.AssemblyContent+traverseEntity@218::GenerateNext([S.P.CoreLib]System.Collections.Generic.IEnumerable`1<FSharp.Compiler.EditorServices.AssemblySymbol>&)][offset 0x000000BB][found Char] Unexpected type on the stack.
-[IL]: Error [StackUnexpected]: : FSharp.Compiler.EditorServices.ParsedInput+visitor@1423-11::VisitExpr([FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<FSharp.Compiler.Syntax.SyntaxNode>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<FSharp.Compiler.Syntax.SynExpr,Microsoft.FSharp.Core.FSharpOption`1<FSharp.Compiler.EditorServices.CompletionContext>>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<FSharp.Compiler.Syntax.SynExpr,Microsoft.FSharp.Core.FSharpOption`1<FSharp.Compiler.EditorServices.CompletionContext>>, [FSharp.Compiler.Service]FSharp.Compiler.Syntax.SynExpr)][offset 0x00000618][found Char] Unexpected type on the stack.
+[IL]: Error [StackUnexpected]: : FSharp.Compiler.EditorServices.ParsedInput+visitor@1423-11::VisitExpr([FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<FSharp.Compiler.Syntax.SyntaxNode>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<FSharp.Compiler.Syntax.SynExpr,Microsoft.FSharp.Core.FSharpOption`1<FSharp.Compiler.EditorServices.CompletionContext>>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<FSharp.Compiler.Syntax.SynExpr,Microsoft.FSharp.Core.FSharpOption`1<FSharp.Compiler.EditorServices.CompletionContext>>, [FSharp.Compiler.Service]FSharp.Compiler.Syntax.SynExpr)][offset 0x00000620][found Char] Unexpected type on the stack.
 [IL]: Error [StackUnexpected]: : <StartupCode$FSharp-Compiler-Service>.$ServiceLexing+clo@921-523::Invoke([FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`3<FSharp.Compiler.Parser+token,int32,int32>,Microsoft.FSharp.Core.Unit>)][offset 0x00000032][found Char] Unexpected type on the stack.
 [IL]: Error [StackUnexpected]: : <StartupCode$FSharp-Compiler-Service>.$ServiceLexing+clo@921-523::Invoke([FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`3<FSharp.Compiler.Parser+token,int32,int32>,Microsoft.FSharp.Core.Unit>)][offset 0x0000003B][found Char] Unexpected type on the stack.
 [IL]: Error [StackUnexpected]: : <StartupCode$FSharp-Compiler-Service>.$ServiceLexing+clo@921-523::Invoke([FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`3<FSharp.Compiler.Parser+token,int32,int32>,Microsoft.FSharp.Core.Unit>)][offset 0x00000064][found Char] Unexpected type on the stack.

ILVerify will always need to run on an up-to-date proto and product compilers, because it always does a clean build in the CI, but not locally if not built from clean state.

So git clean -xdf && pwsh tests/ILVerify/ilverify.ps1 should always do it.

Meanwhile, I've pushed baseline changes.

[Martin521]

Hah, it's green now. Thank you @vzarytovskii.

[psfinaki]

Alright, getting this in. Thanks Martin for dealing with annoying edge cases / races.