Shim/file system: fix leaks of the shim (#18144)

* Tests: fix setting the original shim in the FileSystem shim tests * Shim/file system: fix the shim is captured during the analysis * Release notes * Fantomas
dotnet · Dec 16, 2024 · 3914a47 · 3914a47
1 parent 471fdc8
commit 3914a47
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 9 deletions.
diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
@@ -17,6 +17,7 @@
 * Add missing nullable-metadata for C# consumers of records,exceptions and DU subtypes generated from F# code. [PR #18079](https://github.com/dotnet/fsharp/pull/18079)
 * Fix a race condition in file book keeping in the compiler service ([#18008](https://github.com/dotnet/fsharp/pull/18008))
 * Fix trimming '%' characters when lowering interpolated string to a concat call [PR #18123](https://github.com/dotnet/fsharp/pull/18123)
+* Shim/file system: fix leaks of the shim [PR #18144](https://github.com/dotnet/fsharp/pull/18144)
 
 ### Added
 

diff --git a/src/Compiler/Driver/fsc.fs b/src/Compiler/Driver/fsc.fs
@@ -1129,7 +1129,10 @@ let main6
     DoesNotRequireCompilerThreadTokenAndCouldPossiblyBeMadeConcurrent ctok
 
     let pdbfile =
-        pdbfile |> Option.map (tcConfig.MakePathAbsolute >> FileSystem.GetFullPathShim)
+        pdbfile
+        |> Option.map (fun s ->
+            let absolutePath = tcConfig.MakePathAbsolute s
+            FileSystem.GetFullPathShim(absolutePath))
 
     let normalizeAssemblyRefs (aref: ILAssemblyRef) =
         match tcImports.TryFindDllInfo(ctok, rangeStartup, aref.Name, lookupOnly = false) with

diff --git a/src/Compiler/TypedTree/TcGlobals.fs b/src/Compiler/TypedTree/TcGlobals.fs
@@ -680,8 +680,14 @@ type TcGlobals(
 
   let mkSourceDoc fileName = ILSourceDocument.Create(language=None, vendor=None, documentType=None, file=fileName)
 
+  let compute i =
+      let path = fileOfFileIndex i
+      let fullPath = FileSystem.GetFullFilePathInDirectoryShim directoryToResolveRelativePaths path
+      mkSourceDoc fullPath
+
   // Build the memoization table for files
-  let v_memoize_file = MemoizationTable<int, ILSourceDocument>((fileOfFileIndex >> FileSystem.GetFullFilePathInDirectoryShim directoryToResolveRelativePaths >> mkSourceDoc), keyComparer=HashIdentity.Structural)
+  let v_memoize_file =
+      MemoizationTable<int, ILSourceDocument>(compute, keyComparer = HashIdentity.Structural)
 
   let v_and_info =                   makeIntrinsicValRef(fslib_MFIntrinsicOperators_nleref,                    CompileOpName "&"                      , None                 , None          , [],         mk_rel_sig v_bool_ty)
   let v_addrof_info =                makeIntrinsicValRef(fslib_MFIntrinsicOperators_nleref,                    CompileOpName "~&"                     , None                 , None          , [vara],     ([[varaTy]], mkByrefTy varaTy))

diff --git a/src/LegacyMSBuildResolver/LegacyMSBuildReferenceResolver.fs b/src/LegacyMSBuildResolver/LegacyMSBuildReferenceResolver.fs
@@ -432,7 +432,8 @@ let getResolver () =
                 |]
 
             let rooted, unrooted =
-                references |> Array.partition (fst >> FileSystem.IsPathRootedShim)
+                references
+                |> Array.partition (fun (path, _) -> FileSystem.IsPathRootedShim(path))
 
             let rootedResults =
                 ResolveCore(

diff --git a/tests/FSharp.Compiler.Service.Tests/FileSystemTests.fs b/tests/FSharp.Compiler.Service.Tests/FileSystemTests.fs
@@ -38,17 +38,16 @@ type internal MyFileSystem() =
             | _ -> base.OpenFileForReadShim(filePath, useMemoryMappedFile, shouldShadowCopy)
         override _.FileExistsShim(fileName) = MyFileSystem.FilesCache.ContainsKey(fileName) || base.FileExistsShim(fileName)
 
-let UseMyFileSystem() =
-    let myFileSystem = MyFileSystem()
-    FileSystemAutoOpens.FileSystem <- myFileSystem
-    { new IDisposable with member x.Dispose() = FileSystemAutoOpens.FileSystem <- myFileSystem }
+let useFileSystemShim (shim: IFileSystem) =
+    let originalShim = FileSystem
+    FileSystem <- shim
+    { new IDisposable with member x.Dispose() = FileSystem <- originalShim }
 
 // .NET Core SKIPPED: need to check if these tests can be enabled for .NET Core testing of FSharp.Compiler.Service"
 [<FactForDESKTOP>]
 let ``FileSystem compilation test``() =
   if Environment.OSVersion.Platform = PlatformID.Win32NT then // file references only valid on Windows
-    use myFileSystem =  UseMyFileSystem()
-    let programFilesx86Folder = Environment.GetEnvironmentVariable("PROGRAMFILES(X86)")
+    use myFileSystem = useFileSystemShim (MyFileSystem())
 
     let projectOptions =
         let allFlags =
@@ -83,3 +82,21 @@ let ``FileSystem compilation test``() =
     results.AssemblySignature.Entities.Count |> shouldEqual 2
     results.AssemblySignature.Entities[0].MembersFunctionsAndValues.Count |> shouldEqual 1
     results.AssemblySignature.Entities[0].MembersFunctionsAndValues[0].DisplayName |> shouldEqual "B"
+
+let checkEmptyScriptWithFsShim () =
+    let shim = DefaultFileSystem()
+    let ref: WeakReference = WeakReference(shim)
+
+    use _ = useFileSystemShim shim
+    getParseAndCheckResults "" |> ignore
+
+    ref
+
+[<Fact>]
+let ``File system shim should not leak`` () =
+    let shimRef = checkEmptyScriptWithFsShim ()
+
+    GC.Collect()
+    GC.WaitForPendingFinalizers()
+
+    Assert.False(shimRef.IsAlive)