Skip to content

Commit 3914a47

Browse files
authored
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
1 parent 471fdc8 commit 3914a47

File tree

5 files changed

+37
-9
lines changed

5 files changed

+37
-9
lines changed

docs/release-notes/.FSharp.Compiler.Service/9.0.200.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* 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)
1818
* Fix a race condition in file book keeping in the compiler service ([#18008](https://github.com/dotnet/fsharp/pull/18008))
1919
* Fix trimming '%' characters when lowering interpolated string to a concat call [PR #18123](https://github.com/dotnet/fsharp/pull/18123)
20+
* Shim/file system: fix leaks of the shim [PR #18144](https://github.com/dotnet/fsharp/pull/18144)
2021

2122
### Added
2223

src/Compiler/Driver/fsc.fs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1129,7 +1129,10 @@ let main6
11291129
DoesNotRequireCompilerThreadTokenAndCouldPossiblyBeMadeConcurrent ctok
11301130

11311131
let pdbfile =
1132-
pdbfile |> Option.map (tcConfig.MakePathAbsolute >> FileSystem.GetFullPathShim)
1132+
pdbfile
1133+
|> Option.map (fun s ->
1134+
let absolutePath = tcConfig.MakePathAbsolute s
1135+
FileSystem.GetFullPathShim(absolutePath))
11331136

11341137
let normalizeAssemblyRefs (aref: ILAssemblyRef) =
11351138
match tcImports.TryFindDllInfo(ctok, rangeStartup, aref.Name, lookupOnly = false) with

src/Compiler/TypedTree/TcGlobals.fs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -680,8 +680,14 @@ type TcGlobals(
680680

681681
let mkSourceDoc fileName = ILSourceDocument.Create(language=None, vendor=None, documentType=None, file=fileName)
682682

683+
let compute i =
684+
let path = fileOfFileIndex i
685+
let fullPath = FileSystem.GetFullFilePathInDirectoryShim directoryToResolveRelativePaths path
686+
mkSourceDoc fullPath
687+
683688
// Build the memoization table for files
684-
let v_memoize_file = MemoizationTable<int, ILSourceDocument>((fileOfFileIndex >> FileSystem.GetFullFilePathInDirectoryShim directoryToResolveRelativePaths >> mkSourceDoc), keyComparer=HashIdentity.Structural)
689+
let v_memoize_file =
690+
MemoizationTable<int, ILSourceDocument>(compute, keyComparer = HashIdentity.Structural)
685691

686692
let v_and_info = makeIntrinsicValRef(fslib_MFIntrinsicOperators_nleref, CompileOpName "&" , None , None , [], mk_rel_sig v_bool_ty)
687693
let v_addrof_info = makeIntrinsicValRef(fslib_MFIntrinsicOperators_nleref, CompileOpName "~&" , None , None , [vara], ([[varaTy]], mkByrefTy varaTy))

src/LegacyMSBuildResolver/LegacyMSBuildReferenceResolver.fs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,8 @@ let getResolver () =
432432
|]
433433

434434
let rooted, unrooted =
435-
references |> Array.partition (fst >> FileSystem.IsPathRootedShim)
435+
references
436+
|> Array.partition (fun (path, _) -> FileSystem.IsPathRootedShim(path))
436437

437438
let rootedResults =
438439
ResolveCore(

tests/FSharp.Compiler.Service.Tests/FileSystemTests.fs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,16 @@ type internal MyFileSystem() =
3838
| _ -> base.OpenFileForReadShim(filePath, useMemoryMappedFile, shouldShadowCopy)
3939
override _.FileExistsShim(fileName) = MyFileSystem.FilesCache.ContainsKey(fileName) || base.FileExistsShim(fileName)
4040

41-
let UseMyFileSystem() =
42-
let myFileSystem = MyFileSystem()
43-
FileSystemAutoOpens.FileSystem <- myFileSystem
44-
{ new IDisposable with member x.Dispose() = FileSystemAutoOpens.FileSystem <- myFileSystem }
41+
let useFileSystemShim (shim: IFileSystem) =
42+
let originalShim = FileSystem
43+
FileSystem <- shim
44+
{ new IDisposable with member x.Dispose() = FileSystem <- originalShim }
4545

4646
// .NET Core SKIPPED: need to check if these tests can be enabled for .NET Core testing of FSharp.Compiler.Service"
4747
[<FactForDESKTOP>]
4848
let ``FileSystem compilation test``() =
4949
if Environment.OSVersion.Platform = PlatformID.Win32NT then // file references only valid on Windows
50-
use myFileSystem = UseMyFileSystem()
51-
let programFilesx86Folder = Environment.GetEnvironmentVariable("PROGRAMFILES(X86)")
50+
use myFileSystem = useFileSystemShim (MyFileSystem())
5251

5352
let projectOptions =
5453
let allFlags =
@@ -83,3 +82,21 @@ let ``FileSystem compilation test``() =
8382
results.AssemblySignature.Entities.Count |> shouldEqual 2
8483
results.AssemblySignature.Entities[0].MembersFunctionsAndValues.Count |> shouldEqual 1
8584
results.AssemblySignature.Entities[0].MembersFunctionsAndValues[0].DisplayName |> shouldEqual "B"
85+
86+
let checkEmptyScriptWithFsShim () =
87+
let shim = DefaultFileSystem()
88+
let ref: WeakReference = WeakReference(shim)
89+
90+
use _ = useFileSystemShim shim
91+
getParseAndCheckResults "" |> ignore
92+
93+
ref
94+
95+
[<Fact>]
96+
let ``File system shim should not leak`` () =
97+
let shimRef = checkEmptyScriptWithFsShim ()
98+
99+
GC.Collect()
100+
GC.WaitForPendingFinalizers()
101+
102+
Assert.False(shimRef.IsAlive)

0 commit comments

Comments
 (0)