From ccc9c7b50863ea2835527280da36e71d30143ead Mon Sep 17 00:00:00 2001 From: Eugene Auduchinok Date: Fri, 13 Dec 2024 19:10:04 +0100 Subject: [PATCH 1/4] Tests: fix setting the original shim in the FileSystem shim tests --- .../FSharp.Compiler.Service.Tests/FileSystemTests.fs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/FSharp.Compiler.Service.Tests/FileSystemTests.fs b/tests/FSharp.Compiler.Service.Tests/FileSystemTests.fs index ff62594067d..514b1e22164 100644 --- 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" [] 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 = From 2bb09cdbcc73abf8a645bdc1f3ff63835bf88a76 Mon Sep 17 00:00:00 2001 From: Eugene Auduchinok Date: Fri, 13 Dec 2024 19:23:01 +0100 Subject: [PATCH 2/4] Shim/file system: fix the shim is captured during the analysis --- src/Compiler/Driver/fsc.fs | 5 ++++- src/Compiler/TypedTree/TcGlobals.fs | 8 +++++++- .../LegacyMSBuildReferenceResolver.fs | 2 +- .../FileSystemTests.fs | 18 ++++++++++++++++++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/Compiler/Driver/fsc.fs b/src/Compiler/Driver/fsc.fs index 1d17950a9ac..656b768874c 100644 --- 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 index 0bf99b86572..4f4a45c735e 100644 --- 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((fileOfFileIndex >> FileSystem.GetFullFilePathInDirectoryShim directoryToResolveRelativePaths >> mkSourceDoc), keyComparer=HashIdentity.Structural) + let v_memoize_file = + MemoizationTable(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 index c4230868740..ab5a0f00bd3 100644 --- a/src/LegacyMSBuildResolver/LegacyMSBuildReferenceResolver.fs +++ b/src/LegacyMSBuildResolver/LegacyMSBuildReferenceResolver.fs @@ -432,7 +432,7 @@ 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 index 514b1e22164..e11d97096f8 100644 --- a/tests/FSharp.Compiler.Service.Tests/FileSystemTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/FileSystemTests.fs @@ -82,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 + +[] +let ``File system shim should not leak`` () = + let shimRef = checkEmptyScriptWithFsShim () + + GC.Collect() + GC.WaitForPendingFinalizers() + + Assert.False(shimRef.IsAlive) From 16b4d6402cf0784af31488b438c8d9c0818dcf64 Mon Sep 17 00:00:00 2001 From: Eugene Auduchinok Date: Fri, 13 Dec 2024 21:39:58 +0100 Subject: [PATCH 3/4] Release notes --- docs/release-notes/.FSharp.Compiler.Service/9.0.200.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md index 33125ff07ed..311019abbfe 100644 --- 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 From c427e05671a06086ce8d52644c8bdea377ad6bbb Mon Sep 17 00:00:00 2001 From: Eugene Auduchinok Date: Fri, 13 Dec 2024 21:51:39 +0100 Subject: [PATCH 4/4] Fantomas --- src/Compiler/Driver/fsc.fs | 6 +++--- src/LegacyMSBuildResolver/LegacyMSBuildReferenceResolver.fs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Compiler/Driver/fsc.fs b/src/Compiler/Driver/fsc.fs index 656b768874c..696761f43da 100644 --- a/src/Compiler/Driver/fsc.fs +++ b/src/Compiler/Driver/fsc.fs @@ -1129,10 +1129,10 @@ let main6 DoesNotRequireCompilerThreadTokenAndCouldPossiblyBeMadeConcurrent ctok let pdbfile = - pdbfile |> Option.map (fun s -> + pdbfile + |> Option.map (fun s -> let absolutePath = tcConfig.MakePathAbsolute s - FileSystem.GetFullPathShim(absolutePath) - ) + FileSystem.GetFullPathShim(absolutePath)) let normalizeAssemblyRefs (aref: ILAssemblyRef) = match tcImports.TryFindDllInfo(ctok, rangeStartup, aref.Name, lookupOnly = false) with diff --git a/src/LegacyMSBuildResolver/LegacyMSBuildReferenceResolver.fs b/src/LegacyMSBuildResolver/LegacyMSBuildReferenceResolver.fs index ab5a0f00bd3..54c2f2d32b4 100644 --- a/src/LegacyMSBuildResolver/LegacyMSBuildReferenceResolver.fs +++ b/src/LegacyMSBuildResolver/LegacyMSBuildReferenceResolver.fs @@ -432,7 +432,8 @@ let getResolver () = |] let rooted, unrooted = - references |> Array.partition (fun (path, _) -> FileSystem.IsPathRootedShim(path)) + references + |> Array.partition (fun (path, _) -> FileSystem.IsPathRootedShim(path)) let rootedResults = ResolveCore(