Skip to content

Commit

Permalink
Second half of fix to #106
Browse files Browse the repository at this point in the history
FSharp.Compiler.Service contained a more complete fix for
#106, which relates to the
memory leaks that can arise if type provider instance objects are kept live.
Historically the memory leaks have happened if a type provider registers
itself to listen to file system events (or other notifications), but for some
reason doesn't disconnect those listeners when the type provider is disposed.
This keeps the type provider instance alive - which is a bug in the type
provider - but even worse it can keep '00s MB of data associated with the
compiler state alive as well.

The missing part of the fix is to disconnect the "TypeProviderConfig" callback
when the type providers are disposed, so that even if a type provider instance
hangs on to a reference to the TypeProviderConfig, that doesn't cause the entire
object graph for the compilation session to be retained in memory.

After type provider disposal, any TypeProviderConfig objects provided to the
type provider will now raise ObjectDisposedException when the
SystemRuntimeContainsType method is called.

closes #591
fixes #106

commit d3b2f0e
Author: Don Syme <[email protected]>
Date:   Fri Aug 14 14:35:53 2015 +0100

    testing for 2nd half of 106

commit adebe18
Author: Don Syme <[email protected]>
Date:   Fri Aug 14 14:09:51 2015 +0100

    Second half of fix to #106
  • Loading branch information
dsyme authored and latkin committed Sep 16, 2015
1 parent 9157d60 commit 225b990
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 2 deletions.
11 changes: 10 additions & 1 deletion src/fsharp/CompileOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3837,10 +3837,19 @@ type TcImports(tcConfigP:TcConfigProvider, initialResolutions:TcAssemblyResoluti
referencedAssemblies = [| for r in resolutions.GetAssemblyResolutions() -> r.resolvedPath |]
temporaryFolder = Path.GetTempPath() }

// The type provider should not hold strong references to disposed
// TcImport objects. So the callbacks provided in the type provider config
// dispatch via a thunk which gets set to a non-resource-capturing
// failing function when the object is disposed.
let systemRuntimeContainsType =
let systemRuntimeContainsTypeRef = ref tcImports.SystemRuntimeContainsType
tcImports.AttachDisposeAction(fun () -> systemRuntimeContainsTypeRef := (fun _ -> raise (System.ObjectDisposedException("The type provider has been disposed"))))
fun arg -> systemRuntimeContainsTypeRef.Value arg

let providers =
[ for assemblyName in providerAssemblies do
yield ExtensionTyping.GetTypeProvidersOfAssembly(fileNameOfRuntimeAssembly, ilScopeRefOfRuntimeAssembly, assemblyName, typeProviderEnvironment,
tcConfig.isInvalidationSupported, tcConfig.isInteractive, tcImports.SystemRuntimeContainsType, systemRuntimeAssemblyVersion, m) ]
tcConfig.isInvalidationSupported, tcConfig.isInteractive, systemRuntimeContainsType, systemRuntimeAssemblyVersion, m) ]
let providers = providers |> List.concat

// Note, type providers are disposable objects. The TcImports owns the provider objects - when/if it is disposed, the providers are disposed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,25 @@ module internal TPModule =
module GlobalCounters =
let mutable creations = 0
let mutable disposals = 0
let mutable configs = ([]: TypeProviderConfig list)
let GetTotalCreations() = creations
let GetTotalDisposals() = disposals
let CheckAllConfigsDisposed() =
for c in configs do
try
c.SystemRuntimeContainsType("System.Object") |> ignore
failwith "expected configuration object to be disposed"
with :? System.ObjectDisposedException ->
()



[<TypeProvider>]
type HelloWorldProvider() =
type HelloWorldProvider(config: TypeProviderConfig) =
inherit TypeProviderForNamespaces(TPModule.namespaceName,TPModule.types)
do GlobalCounters.creations <- GlobalCounters.creations + 1
let mutable disposed = false
do GlobalCounters.configs <- config :: GlobalCounters.configs
interface System.IDisposable with
member x.Dispose() =
System.Diagnostics.Debug.Assert(not disposed)
Expand Down
4 changes: 4 additions & 0 deletions vsintegration/src/unittests/Tests.LanguageService.Script.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1628,6 +1628,8 @@ type ScriptTests() as this =
Assert.IsNotNull(totalCreationsMeth, "totalCreationsMeth should not be null")
let totalDisposalsMeth = providerCounters.GetMethod("GetTotalDisposals")
Assert.IsNotNull(totalDisposalsMeth, "totalDisposalsMeth should not be null")
let checkConfigsMeth = providerCounters.GetMethod("CheckAllConfigsDisposed")
Assert.IsNotNull(checkConfigsMeth, "checkConfigsMeth should not be null")

let providerCounters2 = providerAssembly.GetType("Microsoft.FSharp.TypeProvider.Emit.GlobalCountersForInvalidation")
Assert.IsNotNull(providerCounters2, "provider counters #2 module should not be null")
Expand All @@ -1638,6 +1640,7 @@ type ScriptTests() as this =

let totalCreations() = totalCreationsMeth.Invoke(null, [| |]) :?> int
let totalDisposals() = totalDisposalsMeth.Invoke(null, [| |]) :?> int
let checkConfigsDisposed() = checkConfigsMeth.Invoke(null, [| |]) |> ignore
let totalInvaldiationHandlersAdded() = totalInvaldiationHandlersAddedMeth.Invoke(null, [| |]) :?> int
let totalInvaldiationHandlersRemoved() = totalInvaldiationHandlersRemovedMeth.Invoke(null, [| |]) :?> int

Expand Down Expand Up @@ -1693,6 +1696,7 @@ type ScriptTests() as this =
ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients(this.VS)
Assert.IsTrue(countDisposals() = 50, "Check6b, at end, countDisposals() = 50 after explicit clearing")
Assert.IsTrue(countInvaldiationHandlersAdded() - countInvaldiationHandlersRemoved() = 0, "Check6b2, at end, all invalidation handlers removed after explicit cleraring")
checkConfigsDisposed()

[<Test>]
[<Category("TypeProvider")>]
Expand Down

0 comments on commit 225b990

Please sign in to comment.