diff --git a/fcs/FSharp.Compiler.Service/FSharp.Compiler.Service.fsproj b/fcs/FSharp.Compiler.Service/FSharp.Compiler.Service.fsproj index 134a3ca745d..6b2d04cb626 100644 --- a/fcs/FSharp.Compiler.Service/FSharp.Compiler.Service.fsproj +++ b/fcs/FSharp.Compiler.Service/FSharp.Compiler.Service.fsproj @@ -1,4 +1,4 @@ - + @@ -543,18 +543,18 @@ Symbols/SymbolPatterns.fs - - Service/IncrementalBuild.fsi - - - Service/IncrementalBuild.fs - Service/Reactor.fsi Service/Reactor.fs + + Service/IncrementalBuild.fsi + + + Service/IncrementalBuild.fs + Service/ServiceConstants.fs diff --git a/src/fsharp/CompileOps.fs b/src/fsharp/CompileOps.fs index d95bf2d0706..491a753b9d9 100644 --- a/src/fsharp/CompileOps.fs +++ b/src/fsharp/CompileOps.fs @@ -1963,6 +1963,14 @@ type UnresolvedAssemblyReference = UnresolvedAssemblyReference of string * Assem type ResolvedExtensionReference = ResolvedExtensionReference of string * AssemblyReference list * Tainted list #endif +/// The thread in which compilation calls will be enqueued and done work on. +/// Note: This is currently only used when disposing of type providers and will be extended to all the other type provider calls when compilations can be done in parallel. +/// Right now all calls in FCS to type providers are single-threaded through use of the reactor thread. +type ICompilationThread = + + /// Enqueue work to be done on a compilation thread. + abstract EnqueueWork: (CompilationThreadToken -> unit) -> unit + type ImportedBinary = { FileName: string RawMetadata: IRawFSharpAssemblyData @@ -2115,6 +2123,7 @@ type TcConfigBuilder = /// show messages about extension type resolution? mutable showExtensionTypeMessages: bool #endif + mutable compilationThread: ICompilationThread /// pause between passes? mutable pause: bool @@ -2274,6 +2283,9 @@ type TcConfigBuilder = #if !NO_EXTENSIONTYPING showExtensionTypeMessages = false #endif + compilationThread = + let ctok = CompilationThreadToken () + { new ICompilationThread with member __.EnqueueWork work = work ctok } pause = false alwaysCallVirt = true noDebugData = false @@ -2746,8 +2758,9 @@ type TcConfig private (data: TcConfigBuilder, validate: bool) = member x.showLoadedAssemblies = data.showLoadedAssemblies member x.continueAfterParseFailure = data.continueAfterParseFailure #if !NO_EXTENSIONTYPING - member x.showExtensionTypeMessages = data.showExtensionTypeMessages + member x.showExtensionTypeMessages = data.showExtensionTypeMessages #endif + member x.compilationThread = data.compilationThread member x.pause = data.pause member x.alwaysCallVirt = data.alwaysCallVirt member x.noDebugData = data.noDebugData @@ -3741,10 +3754,43 @@ type TcConfigProvider = // TcImports //-------------------------------------------------------------------------- - +#if !NO_EXTENSIONTYPING +// These are hacks in order to allow TcImports to be held as a weak reference inside a type provider. +// The reason is due to older type providers compiled using an older TypeProviderSDK, that SDK used reflection on fields and properties to determine the contract. +// The reflection code has now since been removed, see here: https://github.com/fsprojects/FSharp.TypeProviders.SDK/pull/305. But we still need to work on older type providers. +// One day we can remove these hacks when we deemed most if not all type providers were re-compiled using the newer TypeProviderSDK. +// Yuck. +type TcImportsDllInfoHack = + { + FileName: string + } + +and TcImportsWeakHack (tcImports: WeakReference) = + let mutable dllInfos: TcImportsDllInfoHack list = [] + + member __.SetDllInfos (value: ImportedBinary list) = + dllInfos <- value |> List.map (fun x -> { FileName = x.FileName }) + + member __.Base: TcImportsWeakHack option = + match tcImports.TryGetTarget() with + | true, strong -> + match strong.Base with + | Some (baseTcImports: TcImports) -> + Some baseTcImports.Weak + | _ -> + None + | _ -> + None + + member __.SystemRuntimeContainsType typeName = + match tcImports.TryGetTarget () with + | true, strong -> strong.SystemRuntimeContainsType typeName + | _ -> false +#endif /// Represents a table of imported assemblies with their resolutions. -[] -type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolutions, importsBase: TcImports option, ilGlobalsOpt) = +/// Is a disposable object, but it is recommended not to explicitly call Dispose unless you absolutely know nothing will be using its contents after the disposal. +/// Otherwise, simply allow the GC to collect this and it will properly call Dispose from the finalizer. +and [] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolutions, importsBase: TcImports option, ilGlobalsOpt, compilationThread: ICompilationThread) as this = let mutable resolutions = initialResolutions let mutable importsBase: TcImports option = importsBase @@ -3757,12 +3803,30 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu let mutable ilGlobalsOpt = ilGlobalsOpt let mutable tcGlobals = None #if !NO_EXTENSIONTYPING + let mutable disposeTypeProviderActions = [] let mutable generatedTypeRoots = new System.Collections.Generic.Dictionary() + let mutable tcImportsWeak = TcImportsWeakHack (WeakReference<_> this) #endif let CheckDisposed() = if disposed then assert false + let dispose () = + CheckDisposed() + // disposing deliberately only closes this tcImports, not the ones up the chain + disposed <- true + if verbose then + dprintf "disposing of TcImports, %d binaries\n" disposeActions.Length +#if !NO_EXTENSIONTYPING + let actions = disposeTypeProviderActions + disposeTypeProviderActions <- [] + if actions.Length > 0 then + compilationThread.EnqueueWork (fun _ -> for action in actions do action()) +#endif + let actions = disposeActions + disposeActions <- [] + for action in actions do action() + static let ccuHasType (ccu: CcuThunk) (nsname: string list) (tname: string) = let matchNameSpace (entityOpt: Entity option) n = match entityOpt with @@ -3776,8 +3840,8 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu | Some _ -> true | None -> false | None -> false - - member private tcImports.Base = + + member internal tcImports.Base = CheckDisposed() importsBase @@ -3787,7 +3851,13 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu member tcImports.DllTable = CheckDisposed() - dllTable + dllTable + +#if !NO_EXTENSIONTYPING + member tcImports.Weak = + CheckDisposed() + tcImportsWeak +#endif member tcImports.RegisterCcu ccuInfo = CheckDisposed() @@ -3798,9 +3868,12 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu member tcImports.RegisterDll dllInfo = CheckDisposed() dllInfos <- dllInfos ++ dllInfo +#if !NO_EXTENSIONTYPING + tcImportsWeak.SetDllInfos dllInfos +#endif dllTable <- NameMap.add (getNameOfScopeRef dllInfo.ILScopeRef) dllInfo dllTable - member tcImports.GetDllInfos() = + member tcImports.GetDllInfos() : ImportedBinary list = CheckDisposed() match importsBase with | Some importsBase-> importsBase.GetDllInfos() @ dllInfos @@ -3967,9 +4040,15 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu |> Seq.toList #endif - member tcImports.AttachDisposeAction action = + member private tcImports.AttachDisposeAction action = CheckDisposed() disposeActions <- action :: disposeActions + +#if !NO_EXTENSIONTYPING + member private tcImports.AttachDisposeTypeProviderAction action = + CheckDisposed() + disposeTypeProviderActions <- action :: disposeTypeProviderActions +#endif // Note: the returned binary reader is associated with the tcImports, i.e. when the tcImports are closed // then the reader is closed. @@ -4119,7 +4198,7 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu | _ -> failwith "Unexpected representation in namespace entity referred to by a type provider" - member tcImports.ImportTypeProviderExtensions + member tcImportsStrong.ImportTypeProviderExtensions (ctok, tcConfig: TcConfig, fileNameOfRuntimeAssembly, ilScopeRefOfRuntimeAssembly, @@ -4153,7 +4232,7 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu { resolutionFolder = tcConfig.implicitIncludeDir outputFile = tcConfig.outputFile showResolutionMessages = tcConfig.showExtensionTypeMessages - referencedAssemblies = Array.distinct [| for r in tcImports.AllAssemblyResolutions() -> r.resolvedPath |] + referencedAssemblies = Array.distinct [| for r in tcImportsStrong.AllAssemblyResolutions() -> r.resolvedPath |] temporaryFolder = FileSystem.GetTempPathShim() } // The type provider should not hold strong references to disposed @@ -4161,9 +4240,10 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu // dispatch via a thunk which gets set to a non-resource-capturing // failing function when the object is disposed. let systemRuntimeContainsType = - // NOTE: do not touch this + // NOTE: do not touch this, edit: but we did, we had no choice - TPs cannot hold a strong reference on TcImports "ever". + let tcImports = tcImportsWeak let systemRuntimeContainsTypeRef = ref (fun typeName -> tcImports.SystemRuntimeContainsType typeName) - tcImports.AttachDisposeAction(fun () -> systemRuntimeContainsTypeRef := (fun _ -> raise (System.ObjectDisposedException("The type provider has been disposed")))) + tcImportsStrong.AttachDisposeTypeProviderAction(fun () -> systemRuntimeContainsTypeRef := (fun _ -> raise (System.ObjectDisposedException("The type provider has been disposed")))) fun arg -> systemRuntimeContainsTypeRef.Value arg let providers = @@ -4174,7 +4254,7 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu // Note, type providers are disposable objects. The TcImports owns the provider objects - when/if it is disposed, the providers are disposed. // We ignore all exceptions from provider disposal. for provider in providers do - tcImports.AttachDisposeAction(fun () -> + tcImportsStrong.AttachDisposeTypeProviderAction(fun () -> try provider.PUntaintNoFailure(fun x -> x).Dispose() with e -> @@ -4203,7 +4283,7 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu let handler = tp.Invalidate.Subscribe(fun _ -> capturedInvalidateCcu.Trigger (capturedMessage)) // When the TcImports is disposed we detach the invalidation callback - tcImports.AttachDisposeAction(fun () -> try handler.Dispose() with _ -> ())), m) + tcImportsStrong.AttachDisposeTypeProviderAction(fun () -> try handler.Dispose() with _ -> ())), m) match providers with | [] -> @@ -4222,7 +4302,7 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu // for that namespace. let rec loop (providedNamespace: Tainted) = let path = ExtensionTyping.GetProvidedNamespaceAsPath(m, provider, providedNamespace.PUntaint((fun r -> r.NamespaceName), m)) - tcImports.InjectProvidedNamespaceOrTypeIntoEntity (typeProviderEnvironment, tcConfig, m, entityToInjectInto, [], path, provider, None) + tcImportsStrong.InjectProvidedNamespaceOrTypeIntoEntity (typeProviderEnvironment, tcConfig, m, entityToInjectInto, [], path, provider, None) // Inject entities for the types returned by provider.GetTypes(). // @@ -4232,7 +4312,7 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu let tys = providedNamespace.PApplyArray((fun provider -> provider.GetTypes()), "GetTypes", m) let ptys = [| for ty in tys -> ty.PApply((fun ty -> ty |> ProvidedType.CreateNoContext), m) |] for st in ptys do - tcImports.InjectProvidedNamespaceOrTypeIntoEntity (typeProviderEnvironment, tcConfig, m, entityToInjectInto, [], path, provider, Some st) + tcImportsStrong.InjectProvidedNamespaceOrTypeIntoEntity (typeProviderEnvironment, tcConfig, m, entityToInjectInto, [], path, provider, Some st) for providedNestedNamespace in providedNamespace.PApplyArray((fun provider -> provider.GetNestedNamespaces()), "GetNestedNamespaces", m) do loop providedNestedNamespace @@ -4564,9 +4644,7 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu // Note: This returns a TcImports object. However, framework TcImports are not currently disposed. The only reason // we dispose TcImports is because we need to dispose type providers, and type providers are never included in the framework DLL set. - // - // If this ever changes then callers may need to begin disposing the TcImports (though remember, not before all derived - // non-framework TcImports built related to this framework TcImports are disposed). + // If a framework set ever includes type providers, you will not have to worry about explicitly calling Dispose as the Finalizer will handle it. static member BuildFrameworkTcImports (ctok, tcConfigP: TcConfigProvider, frameworkDLLs, nonFrameworkDLLs) = cancellable { @@ -4574,8 +4652,7 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu let tcResolutions = TcAssemblyResolutions.BuildFromPriorResolutions(ctok, tcConfig, frameworkDLLs, []) let tcAltResolutions = TcAssemblyResolutions.BuildFromPriorResolutions(ctok, tcConfig, nonFrameworkDLLs, []) - // Note: TcImports are disposable - the caller owns this object and must dispose - let frameworkTcImports = new TcImports(tcConfigP, tcResolutions, None, None) + let frameworkTcImports = new TcImports(tcConfigP, tcResolutions, None, None, tcConfig.compilationThread) // Fetch the primaryAssembly from the referenced assemblies otherwise let primaryAssemblyReference = @@ -4666,24 +4743,21 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu knownUnresolved |> List.map (function UnresolvedAssemblyReference(file, originalReferences) -> file, originalReferences) |> List.iter reportAssemblyNotResolved + + override tcImports.Finalize () = + dispose () - // Note: This returns a TcImports object. TcImports are disposable - the caller owns the returned TcImports object - // and when hosted in Visual Studio or another long-running process must dispose this object. static member BuildNonFrameworkTcImports (ctok, tcConfigP: TcConfigProvider, tcGlobals: TcGlobals, baseTcImports, nonFrameworkReferences, knownUnresolved) = cancellable { let tcConfig = tcConfigP.Get ctok let tcResolutions = TcAssemblyResolutions.BuildFromPriorResolutions(ctok, tcConfig, nonFrameworkReferences, knownUnresolved) let references = tcResolutions.GetAssemblyResolutions() - let tcImports = new TcImports(tcConfigP, tcResolutions, Some baseTcImports, Some tcGlobals.ilg) + let tcImports = new TcImports(tcConfigP, tcResolutions, Some baseTcImports, Some tcGlobals.ilg, tcConfig.compilationThread) let! _assemblies = tcImports.RegisterAndImportReferencedAssemblies(ctok, references) tcImports.ReportUnresolvedAssemblyReferences knownUnresolved return tcImports } - // Note: This returns a TcImports object. TcImports are disposable - the caller owns the returned TcImports object - // and if hosted in Visual Studio or another long-running process must dispose this object. However this - // function is currently only used from fsi.exe. If we move to a long-running hosted evaluation service API then - // we should start disposing these objects. static member BuildTcImports(ctok, tcConfigP: TcConfigProvider) = cancellable { let tcConfig = tcConfigP.Get ctok @@ -4696,14 +4770,8 @@ type TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAssemblyResolu interface System.IDisposable with member tcImports.Dispose() = - CheckDisposed() - // disposing deliberately only closes this tcImports, not the ones up the chain - disposed <- true - if verbose then - dprintf "disposing of TcImports, %d binaries\n" disposeActions.Length - let actions = disposeActions - disposeActions <- [] - for action in actions do action() + dispose () + GC.SuppressFinalize tcImports override tcImports.ToString() = "TcImports(...)" diff --git a/src/fsharp/CompileOps.fsi b/src/fsharp/CompileOps.fsi index 3d08610670a..71bb7c6f8b6 100644 --- a/src/fsharp/CompileOps.fsi +++ b/src/fsharp/CompileOps.fsi @@ -209,6 +209,14 @@ type UnresolvedAssemblyReference = UnresolvedAssemblyReference of string * Assem type ResolvedExtensionReference = ResolvedExtensionReference of string * AssemblyReference list * Tainted list #endif +/// The thread in which compilation calls will be enqueued and done work on. +/// Note: This is currently only used when disposing of type providers and will be extended to all the other type provider calls when compilations can be done in parallel. +/// Right now all calls in FCS to type providers are single-threaded through use of the reactor thread. +type ICompilationThread = + + /// Enqueue work to be done on a compilation thread. + abstract EnqueueWork: (CompilationThreadToken -> unit) -> unit + [] type CompilerTarget = | WinExe @@ -351,6 +359,7 @@ type TcConfigBuilder = #if !NO_EXTENSIONTYPING mutable showExtensionTypeMessages: bool #endif + mutable compilationThread: ICompilationThread mutable pause: bool mutable alwaysCallVirt: bool mutable noDebugData: bool @@ -510,6 +519,7 @@ type TcConfig = #if !NO_EXTENSIONTYPING member showExtensionTypeMessages: bool #endif + member compilationThread: ICompilationThread member pause: bool member alwaysCallVirt: bool member noDebugData: bool @@ -591,6 +601,8 @@ type TcAssemblyResolutions = static member BuildFromPriorResolutions : CompilationThreadToken * TcConfig * AssemblyResolution list * UnresolvedAssemblyReference list -> TcAssemblyResolutions /// Represents a table of imported assemblies with their resolutions. +/// Is a disposable object, but it is recommended not to explicitly call Dispose unless you absolutely know nothing will be using its contents after the disposal. +/// Otherwise, simply allow the GC to collect this and it will properly call Dispose from the finalizer. [] type TcImports = interface System.IDisposable diff --git a/src/fsharp/FSharp.Compiler.Private/FSharp.Compiler.Private.fsproj b/src/fsharp/FSharp.Compiler.Private/FSharp.Compiler.Private.fsproj index ebef5fb040b..3b3d38409f7 100644 --- a/src/fsharp/FSharp.Compiler.Private/FSharp.Compiler.Private.fsproj +++ b/src/fsharp/FSharp.Compiler.Private/FSharp.Compiler.Private.fsproj @@ -560,6 +560,12 @@ Symbols/SymbolPatterns.fs + + Service/Reactor.fsi + + + Service/Reactor.fs + @@ -568,12 +574,6 @@ Service/IncrementalBuild.fs - - Service/Reactor.fsi - - - Service/Reactor.fs - Service/ServiceConstants.fs diff --git a/src/fsharp/service/IncrementalBuild.fs b/src/fsharp/service/IncrementalBuild.fs index 24769360aeb..4099b59c803 100755 --- a/src/fsharp/service/IncrementalBuild.fs +++ b/src/fsharp/service/IncrementalBuild.fs @@ -1211,11 +1211,14 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput keepAssemblyContents, keepAllBackgroundResolutions, maxTimeShareMilliseconds) = let tcConfigP = TcConfigProvider.Constant tcConfig - let importsInvalidated = new Event() let fileParsed = new Event() let beforeFileChecked = new Event() let fileChecked = new Event() let projectChecked = new Event() +#if !NO_EXTENSIONTYPING + let importsInvalidatedByTypeProvider = new Event() +#endif + let mutable currentTcImportsOpt = None // Check for the existence of loaded sources and prepend them to the sources list if present. let sourceFiles = tcConfig.GetAvailableLoadedSources() @ (sourceFiles |>List.map (fun s -> rangeStartup, s)) @@ -1242,27 +1245,6 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput for (_, f, _) in sourceFiles do yield f |] - // The IncrementalBuilder needs to hold up to one item that needs to be disposed, which is the tcImports for the incremental - // build. - let mutable cleanupItem = None: TcImports option - let disposeCleanupItem() = - match cleanupItem with - | None -> () - | Some item -> - cleanupItem <- None - dispose item - - let setCleanupItem x = - assert cleanupItem.IsNone - cleanupItem <- Some x - - let mutable disposed = false - let assertNotDisposed() = - if disposed then - System.Diagnostics.Debug.Assert(false, "IncrementalBuild object has already been disposed!") - - let mutable referenceCount = 0 - //---------------------------------------------------- // START OF BUILD TASK FUNCTIONS @@ -1270,14 +1252,12 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput /// /// Get the timestamp of the given file name. let StampFileNameTask (cache: TimeStampCache) _ctok (_m: range, filename: string, _isLastCompiland) = - assertNotDisposed() cache.GetFileTimeStamp filename /// This is a build task function that gets placed into the build rules as the computation for a VectorMap /// /// Parse the given file and return the given input. let ParseTask ctok (sourceRange: range, filename: string, isLastCompiland) = - assertNotDisposed() DoesNotRequireCompilerThreadTokenAndCouldPossiblyBeMadeConcurrent ctok let errorLogger = CompilationErrorLogger("ParseTask", tcConfig.errorSeverityOptions) @@ -1300,7 +1280,6 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput /// /// Timestamps of referenced assemblies are taken from the file's timestamp. let StampReferencedAssemblyTask (cache: TimeStampCache) ctok (_ref, timeStamper) = - assertNotDisposed() timeStamper cache ctok @@ -1309,7 +1288,6 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput // Link all the assemblies together and produce the input typecheck accumulator let CombineImportedAssembliesTask ctok _ : Cancellable = cancellable { - assertNotDisposed() let errorLogger = CompilationErrorLogger("CombineImportedAssembliesTask", tcConfig.errorSeverityOptions) // Return the disposable object that cleans up use _holder = new CompilationGlobalsScope(errorLogger, BuildPhase.Parameter) @@ -1317,10 +1295,6 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput let! tcImports = cancellable { try - // We dispose any previous tcImports, for the case where a dependency changed which caused this part - // of the partial build to be re-evaluated. - disposeCleanupItem() - let! tcImports = TcImports.BuildNonFrameworkTcImports(ctok, tcConfigP, tcGlobals, frameworkTcImports, nonFrameworkResolutions, unresolvedReferences) #if !NO_EXTENSIONTYPING tcImports.GetCcusExcludingBase() |> Seq.iter (fun ccu -> @@ -1338,19 +1312,13 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput // // In the invalidation handler we use a weak reference to allow the IncrementalBuilder to // be collected if, for some reason, a TP instance is not disposed or not GC'd. - let capturedImportsInvalidated = WeakReference<_>(importsInvalidated) + let capturedImportsInvalidated = WeakReference<_>(importsInvalidatedByTypeProvider) ccu.Deref.InvalidateEvent.Add(fun msg -> match capturedImportsInvalidated.TryGetTarget() with | true, tg -> tg.Trigger msg - | _ -> ())) + | _ -> ())) #endif - - - // The tcImports must be cleaned up if this builder ever gets disposed. We also dispose any previous - // tcImports should we be re-creating an entry because a dependency changed which caused this part - // of the partial build to be re-evaluated. - setCleanupItem tcImports - + currentTcImportsOpt <- Some tcImports return tcImports with e -> System.Diagnostics.Debug.Assert(false, sprintf "Could not BuildAllReferencedDllTcImports %A" e) @@ -1390,7 +1358,6 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput /// /// Type check all files. let TypeCheckTask ctok (tcAcc: TypeCheckAccumulator) input: Eventually = - assertNotDisposed() match input with | Some input, _sourceRange, filename, parseErrors-> IncrementalBuilderEventTesting.MRU.Add(IncrementalBuilderEventTesting.IBETypechecked filename) @@ -1462,7 +1429,6 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput /// Finish up the typechecking to produce outputs for the rest of the compilation process let FinalizeTypeCheckTask ctok (tcStates: TypeCheckAccumulator[]) = cancellable { - assertNotDisposed() DoesNotRequireCompilerThreadTokenAndCouldPossiblyBeMadeConcurrent ctok let errorLogger = CompilationErrorLogger("CombineImportedAssembliesTask", tcConfig.errorSeverityOptions) @@ -1581,21 +1547,7 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput partialBuild <- b let MaxTimeStampInDependencies cache (ctok: CompilationThreadToken) (output: INode) = - IncrementalBuild.MaxTimeStampInDependencies cache ctok output.Name partialBuild - - member this.IncrementUsageCount() = - assertNotDisposed() - System.Threading.Interlocked.Increment(&referenceCount) |> ignore - { new System.IDisposable with member __.Dispose() = this.DecrementUsageCount() } - - member __.DecrementUsageCount() = - assertNotDisposed() - let currentValue = System.Threading.Interlocked.Decrement(&referenceCount) - if currentValue = 0 then - disposed <- true - disposeCleanupItem() - - member __.IsAlive = referenceCount > 0 + IncrementalBuild.MaxTimeStampInDependencies cache ctok output.Name partialBuild member __.TcConfig = tcConfig @@ -1607,21 +1559,14 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput member __.ProjectChecked = projectChecked.Publish - member __.ImportedCcusInvalidated = importsInvalidated.Publish - - member __.AllDependenciesDeprecated = allDependencies - #if !NO_EXTENSIONTYPING - member __.ThereAreLiveTypeProviders = - let liveTPs = - match cleanupItem with - | None -> [] - | Some tcImports -> [for ia in tcImports.GetImportedAssemblies() do yield! ia.TypeProviders] - match liveTPs with - | [] -> false - | _ -> true + member __.ImportsInvalidatedByTypeProvider = importsInvalidatedByTypeProvider.Publish #endif + member __.TryGetCurrentTcImports () = currentTcImportsOpt + + member __.AllDependenciesDeprecated = allDependencies + member __.Step (ctok: CompilationThreadToken) = cancellable { let cache = TimeStampCache defaultTimeStamp // One per step @@ -1808,6 +1753,14 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput // Never open PDB files for the language service, even if --standalone is specified tcConfigB.openDebugInformationForLaterStaticLinking <- false + tcConfigB.compilationThread <- + { new ICompilationThread with + member __.EnqueueWork work = + Reactor.Singleton.EnqueueOp ("Unknown", "ICompilationThread.EnqueueWork", "work", fun ctok -> + work ctok + ) + } + tcConfigB, sourceFilesNew match loadClosureOpt with @@ -1887,11 +1840,3 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput return builderOpt, diagnostics } - - static member KeepBuilderAlive (builderOpt: IncrementalBuilder option) = - match builderOpt with - | Some builder -> builder.IncrementUsageCount() - | None -> { new System.IDisposable with member __.Dispose() = () } - - member __.IsBeingKeptAliveApartFromCacheEntry = (referenceCount >= 2) - diff --git a/src/fsharp/service/IncrementalBuild.fsi b/src/fsharp/service/IncrementalBuild.fsi index 89d143ca985..96d00636887 100755 --- a/src/fsharp/service/IncrementalBuild.fsi +++ b/src/fsharp/service/IncrementalBuild.fsi @@ -80,9 +80,6 @@ type internal PartialCheckResults = [] type internal IncrementalBuilder = - /// Check if the builder is not disposed - member IsAlive : bool - /// The TcConfig passed in to the builder creation. member TcConfig : TcConfig @@ -104,15 +101,17 @@ type internal IncrementalBuilder = /// overall analysis results for the project will be quick. member ProjectChecked : IEvent +#if !NO_EXTENSIONTYPING /// Raised when a type provider invalidates the build. - member ImportedCcusInvalidated : IEvent + member ImportsInvalidatedByTypeProvider : IEvent +#endif + + /// Tries to get the current successful TcImports. This is only used in testing. Do not use it for other stuff. + member TryGetCurrentTcImports : unit -> TcImports option /// The list of files the build depends on member AllDependenciesDeprecated : string[] -#if !NO_EXTENSIONTYPING - /// Whether there are any 'live' type providers that may need a refresh when a project is Cleaned - member ThereAreLiveTypeProviders : bool -#endif + /// Perform one step in the F# build. Return true if the background work is finished. member Step : CompilationThreadToken -> Cancellable @@ -164,13 +163,6 @@ type internal IncrementalBuilder = static member TryCreateBackgroundBuilderForProjectOptions : CompilationThreadToken * ReferenceResolver.Resolver * defaultFSharpBinariesDir: string * FrameworkImportsCache * scriptClosureOptions:LoadClosure option * sourceFiles:string list * commandLineArgs:string list * projectReferences: IProjectReference list * projectDirectory:string * useScriptResolutionRules:bool * keepAssemblyContents: bool * keepAllBackgroundResolutions: bool * maxTimeShareMilliseconds: int64 * tryGetMetadataSnapshot: ILBinaryReader.ILReaderTryGetMetadataSnapshot * suggestNamesForErrors: bool -> Cancellable - /// Increment the usage count on the IncrementalBuilder by 1. This initial usage count is 0 so immediately after creation - /// a call to KeepBuilderAlive should be made. The returns an IDisposable which will - /// decrement the usage count and dispose if the usage count goes to zero - static member KeepBuilderAlive : IncrementalBuilder option -> IDisposable - - member IsBeingKeptAliveApartFromCacheEntry : bool - /// Generalized Incremental Builder. This is exposed only for unittesting purposes. module internal IncrementalBuild = type INode = diff --git a/src/fsharp/service/ServiceDeclarationLists.fs b/src/fsharp/service/ServiceDeclarationLists.fs index f06fc56ca2e..c4bf83bf087 100644 --- a/src/fsharp/service/ServiceDeclarationLists.fs +++ b/src/fsharp/service/ServiceDeclarationLists.fs @@ -492,19 +492,12 @@ type FSharpDeclarationListItem(name: string, nameInCode: string, fullName: strin member __.StructuredDescriptionTextAsync = let userOpName = "ToolTip" match info with - | Choice1Of2 (items: CompletionItem list, infoReader, m, denv, reactor:IReactorOperations, checkAlive) -> + | Choice1Of2 (items: CompletionItem list, infoReader, m, denv, reactor:IReactorOperations) -> // reactor causes the lambda to execute on the background compiler thread, through the Reactor reactor.EnqueueAndAwaitOpAsync (userOpName, "StructuredDescriptionTextAsync", name, fun ctok -> RequireCompilationThread ctok - // This is where we do some work which may touch TAST data structures owned by the IncrementalBuilder - infoReader, item etc. - // It is written to be robust to a disposal of an IncrementalBuilder, in which case it will just return the empty string. - // It is best to think of this as a "weak reference" to the IncrementalBuilder, i.e. this code is written to be robust to its - // disposal. Yes, you are right to scratch your head here, but this is ok. - cancellable.Return( - if checkAlive() then - FSharpToolTipText(items |> List.map (fun x -> SymbolHelpers.FormatStructuredDescriptionOfItem true infoReader m denv x.ItemWithInst)) - else - FSharpToolTipText [ FSharpStructuredToolTipElement.Single(wordL (tagText (FSComp.SR.descriptionUnavailable())), FSharpXmlDoc.None) ])) + cancellable.Return(FSharpToolTipText(items |> List.map (fun x -> SymbolHelpers.FormatStructuredDescriptionOfItem true infoReader m denv x.ItemWithInst))) + ) | Choice2Of2 result -> async.Return result @@ -559,7 +552,7 @@ type FSharpDeclarationListInfo(declarations: FSharpDeclarationListItem[], isForT member __.IsError = isError // Make a 'Declarations' object for a set of selected items - static member Create(infoReader:InfoReader, m, denv, getAccessibility, items: CompletionItem list, reactor, currentNamespaceOrModule: string[] option, isAttributeApplicationContext: bool, checkAlive) = + static member Create(infoReader:InfoReader, m: range, denv, getAccessibility, items: CompletionItem list, reactor, currentNamespaceOrModule: string[] option, isAttributeApplicationContext: bool) = let g = infoReader.g let isForType = items |> List.exists (fun x -> x.Type.IsSome) let items = items |> SymbolHelpers.RemoveExplicitlySuppressedCompletionItems g @@ -697,7 +690,7 @@ type FSharpDeclarationListInfo(declarations: FSharpDeclarationListItem[], isForT | ns -> Some (System.String.Join(".", ns))) FSharpDeclarationListItem( - name, nameInCode, fullName, glyph, Choice1Of2 (items, infoReader, m, denv, reactor, checkAlive), getAccessibility item.Item, + name, nameInCode, fullName, glyph, Choice1Of2 (items, infoReader, m, denv, reactor), getAccessibility item.Item, item.Kind, item.IsOwnMember, item.MinorPriority, item.Unresolved.IsNone, namespaceToOpen)) new FSharpDeclarationListInfo(Array.ofList decls, isForType, false) diff --git a/src/fsharp/service/ServiceDeclarationLists.fsi b/src/fsharp/service/ServiceDeclarationLists.fsi index 4dc7905251d..ae8171bfb00 100644 --- a/src/fsharp/service/ServiceDeclarationLists.fsi +++ b/src/fsharp/service/ServiceDeclarationLists.fsi @@ -67,7 +67,7 @@ type public FSharpDeclarationListInfo = member IsError : bool // Implementation details used by other code in the compiler - static member internal Create : infoReader:InfoReader * m:range * denv:DisplayEnv * getAccessibility:(Item -> FSharpAccessibility option) * items:CompletionItem list * reactor:IReactorOperations * currentNamespace:string[] option * isAttributeApplicationContex:bool * checkAlive:(unit -> bool) -> FSharpDeclarationListInfo + static member internal Create : infoReader:InfoReader * m:range * denv:DisplayEnv * getAccessibility:(Item -> FSharpAccessibility option) * items:CompletionItem list * reactor:IReactorOperations * currentNamespace:string[] option * isAttributeApplicationContex:bool -> FSharpDeclarationListInfo static member internal Error : message:string -> FSharpDeclarationListInfo diff --git a/src/fsharp/service/service.fs b/src/fsharp/service/service.fs index 3ca4f8a248f..a63fdccee89 100644 --- a/src/fsharp/service/service.fs +++ b/src/fsharp/service/service.fs @@ -161,7 +161,6 @@ type TypeCheckInfo sFallback: NameResolutionEnv, loadClosure : LoadClosure option, reactorOps : IReactorOperations, - checkAlive : (unit -> bool), textSnapshotInfo:obj option, implFileOpt: TypedImplFile option, openDeclarations: OpenDeclaration[]) = @@ -937,7 +936,7 @@ type TypeCheckInfo |> Option.bind (fun x -> x.ParseTree) |> Option.map (fun parsedInput -> UntypedParseImpl.GetFullNameOfSmallestModuleOrNamespaceAtPoint(parsedInput, mkPos line 0)) let isAttributeApplication = ctx = Some CompletionContext.AttributeApplication - FSharpDeclarationListInfo.Create(infoReader,m,denv,getAccessibility,items,reactorOps,currentNamespaceOrModule,isAttributeApplication,checkAlive)) + FSharpDeclarationListInfo.Create(infoReader,m,denv,getAccessibility,items,reactorOps,currentNamespaceOrModule,isAttributeApplication)) (fun msg -> Trace.TraceInformation(sprintf "FCS: recovering from error in GetDeclarations: '%s'" msg) FSharpDeclarationListInfo.Error msg) @@ -1616,8 +1615,6 @@ module internal Parser = // These are the errors and warnings seen by the background compiler for the entire antecedent backgroundDiagnostics: (PhasedDiagnostic * FSharpErrorSeverity)[], reactorOps: IReactorOperations, - // Used by 'FSharpDeclarationListInfo' to check the IncrementalBuilder is still alive. - checkAlive : (unit -> bool), textSnapshotInfo : obj option, userOpName: string, suggestNamesForErrors: bool) = @@ -1761,7 +1758,6 @@ module internal Parser = tcEnvAtEnd.NameEnv, loadClosure, reactorOps, - checkAlive, textSnapshotInfo, List.tryHead implFiles, sink.GetOpenDeclarations()) @@ -1935,48 +1931,18 @@ type FSharpCheckProjectResults(projectFileName:string, tcConfigOption, keepAssem override info.ToString() = "FSharpCheckProjectResults(" + projectFileName + ")" [] -/// A live object of this type keeps the background corresponding background builder (and type providers) alive (through reference-counting). -// -// There is an important property of all the objects returned by the methods of this type: they do not require -// the corresponding background builder to be alive. That is, they are simply plain-old-data through pre-formatting of all result text. type FSharpCheckFileResults(filename: string, errors: FSharpErrorInfo[], scopeOptX: TypeCheckInfo option, dependencyFiles: string[], builderX: IncrementalBuilder option, reactorOpsX:IReactorOperations, keepAssemblyContents: bool) = - // This may be None initially, or may be set to None when the object is disposed or finalized + // This may be None initially. let mutable details = match scopeOptX with None -> None | Some scopeX -> Some (scopeX, builderX, reactorOpsX) - // Increment the usage count on the IncrementalBuilder. We want to keep the IncrementalBuilder and all associated - // resources and type providers alive for the duration of the lifetime of this object. - let decrementer = - match details with - | Some (_,builderOpt,_) -> IncrementalBuilder.KeepBuilderAlive builderOpt - | _ -> { new System.IDisposable with member x.Dispose() = () } - - let mutable disposed = false - - let dispose() = - if not disposed then - disposed <- true - match details with - | Some (_,_,reactor) -> - // Make sure we run disposal in the reactor thread, since it may trigger type provider disposals etc. - details <- None - reactor.EnqueueOp ("GCFinalizer","FSharpCheckFileResults.DecrementUsageCountOnIncrementalBuilder", filename, fun ctok -> - RequireCompilationThread ctok - decrementer.Dispose()) - | _ -> () - // Run an operation that needs to access a builder and be run in the reactor thread let reactorOp userOpName opName dflt f = async { match details with | None -> return dflt - | Some (_, Some builder, _) when not builder.IsAlive -> - System.Diagnostics.Debug.Assert(false,"unexpected dead builder") - return dflt - | Some (scope, builderOpt, reactor) -> - // Increment the usage count to ensure the builder doesn't get released while running operations asynchronously. - use _unwind = IncrementalBuilder.KeepBuilderAlive builderOpt + | Some (scope, _, reactor) -> let! res = reactor.EnqueueAndAwaitOpAsync(userOpName, opName, filename, fun ctok -> f ctok scope |> cancellable.Return) return res } @@ -1987,13 +1953,14 @@ type FSharpCheckFileResults(filename: string, errors: FSharpErrorInfo[], scopeOp | None -> dflt() | Some (scope, _builderOpt, _ops) -> f scope - // At the moment we only dispose on finalize - we never explicitly dispose these objects. Explicitly disposing is not - // really worth much since the underlying project builds are likely to still be in the incrementalBuilder cache. - override info.Finalize() = dispose() - member info.Errors = errors member info.HasFullTypeCheckInfo = details.IsSome + + member info.TryGetCurrentTcImports () = + match builderX with + | Some builder -> builder.TryGetCurrentTcImports () + | _ -> None /// Intellisense autocompletions member info.GetDeclarationListInfo(parseResultsOpt, line, lineStr, partialName, ?getAllEntities, ?hasTextChangedSinceLastTypecheck, ?userOpName: string) = @@ -2388,17 +2355,16 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC options.UseScriptResolutionRules, keepAssemblyContents, keepAllBackgroundResolutions, maxTimeShareMilliseconds, tryGetMetadataSnapshot, suggestNamesForErrors) - // We're putting the builder in the cache, so increment its count. - let decrement = IncrementalBuilder.KeepBuilderAlive builderOpt - match builderOpt with | None -> () | Some builder -> +#if !NO_EXTENSIONTYPING // Register the behaviour that responds to CCUs being invalidated because of type // provider Invalidate events. This invalidates the configuration in the build. - builder.ImportedCcusInvalidated.Add (fun _ -> + builder.ImportsInvalidatedByTypeProvider.Add (fun _ -> self.InvalidateConfiguration(options, None, userOpName)) +#endif // Register the callback called just before a file is typechecked by the background builder (without recording // errors or intellisense information). @@ -2410,7 +2376,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC builder.FileChecked.Add (fun file -> fileChecked.Trigger(file, options.ExtraProjectInfo)) builder.ProjectChecked.Add (fun () -> projectChecked.Trigger (options.ProjectFileName, options.ExtraProjectInfo)) - return (builderOpt, diagnostics, decrement) + return (builderOpt, diagnostics) } // STATIC ROOT: FSharpLanguageServiceTestable.FSharpChecker.backgroundCompiler.incrementalBuildersCache. This root typically holds more @@ -2419,27 +2385,23 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC // /// Cache of builds keyed by options. let incrementalBuildersCache = - MruCache + MruCache (keepStrongly=projectCacheSize, keepMax=projectCacheSize, areSame = FSharpProjectOptions.AreSameForChecking, - areSimilar = FSharpProjectOptions.UseSameProject, - requiredToKeep=(fun (builderOpt,_,_) -> match builderOpt with None -> false | Some (b:IncrementalBuilder) -> b.IsBeingKeptAliveApartFromCacheEntry), - onDiscard = (fun (_, _, decrement:IDisposable) -> decrement.Dispose())) + areSimilar = FSharpProjectOptions.UseSameProject) - let getOrCreateBuilderAndKeepAlive (ctok, options, userOpName) = + let getOrCreateBuilder (ctok, options, userOpName) = cancellable { RequireCompilationThread ctok match incrementalBuildersCache.TryGet (ctok, options) with - | Some (builderOpt,creationErrors,_) -> + | Some (builderOpt,creationErrors) -> Logger.Log LogCompilerFunctionId.Service_IncrementalBuildersCache_GettingCache - let decrement = IncrementalBuilder.KeepBuilderAlive builderOpt - return builderOpt,creationErrors, decrement + return builderOpt,creationErrors | None -> Logger.Log LogCompilerFunctionId.Service_IncrementalBuildersCache_BuildingNewCache - let! (builderOpt,creationErrors,_) as info = CreateOneIncrementalBuilder (ctok, options, userOpName) + let! (builderOpt,creationErrors) as info = CreateOneIncrementalBuilder (ctok, options, userOpName) incrementalBuildersCache.Set (ctok, options, info) - let decrement = IncrementalBuilder.KeepBuilderAlive builderOpt - return builderOpt, creationErrors, decrement + return builderOpt, creationErrors } let parseCacheLock = Lock() @@ -2530,8 +2492,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC member bc.GetBackgroundParseResultsForFileInProject(filename, options, userOpName) = reactor.EnqueueAndAwaitOpAsync(userOpName, "GetBackgroundParseResultsForFileInProject ", filename, fun ctok -> cancellable { - let! builderOpt, creationErrors, decrement = getOrCreateBuilderAndKeepAlive (ctok, options, userOpName) - use _unwind = decrement + let! builderOpt, creationErrors = getOrCreateBuilder (ctok, options, userOpName) match builderOpt with | None -> return FSharpParseFileResults(creationErrors, None, true, [| |]) | Some builder -> @@ -2600,7 +2561,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC let loadClosure = scriptClosureCacheLock.AcquireLock (fun ltok -> scriptClosureCache.TryGet (ltok, options)) let! tcErrors, tcFileResult = Parser.CheckOneFile(parseResults, sourceText, fileName, options.ProjectFileName, tcPrior.TcConfig, tcPrior.TcGlobals, tcPrior.TcImports, - tcPrior.TcState, tcPrior.ModuleNamesDict, loadClosure, tcPrior.TcErrors, reactorOps, (fun () -> builder.IsAlive), textSnapshotInfo, userOpName, suggestNamesForErrors) + tcPrior.TcState, tcPrior.ModuleNamesDict, loadClosure, tcPrior.TcErrors, reactorOps, textSnapshotInfo, userOpName, suggestNamesForErrors) let parsingOptions = FSharpParsingOptions.FromTcConfig(tcPrior.TcConfig, Array.ofList builder.SourceFiles, options.UseScriptResolutionRules) let checkAnswer = MakeCheckFileAnswer(fileName, tcFileResult, options, builder, Array.ofList tcPrior.TcDependencyFiles, creationErrors, parseResults.Errors, tcErrors) bc.RecordTypeCheckFileInProjectResults(fileName, options, parsingOptions, parseResults, fileVersion, tcPrior.TimeStamp, Some checkAnswer, sourceText.GetHashCode()) @@ -2629,11 +2590,10 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC let! cachedResults = execWithReactorAsync <| fun ctok -> cancellable { - let! _builderOpt,_creationErrors,decrement = getOrCreateBuilderAndKeepAlive (ctok, options, userOpName) - use _unwind = decrement + let! _builderOpt,_creationErrors = getOrCreateBuilder (ctok, options, userOpName) match incrementalBuildersCache.TryGetAny (ctok, options) with - | Some (Some builder, creationErrors, _) -> + | Some (Some builder, creationErrors) -> match bc.GetCachedCheckFileResult(builder, filename, sourceText, options) with | Some (_, checkResults) -> return Some (builder, creationErrors, Some (FSharpCheckFileAnswer.Succeeded checkResults)) | _ -> return Some (builder, creationErrors, None) @@ -2668,8 +2628,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC try if implicitlyStartBackgroundWork then reactor.CancelBackgroundOp() // cancel the background work, since we will start new work after we're done - let! builderOpt,creationErrors, decrement = execWithReactorAsync (fun ctok -> getOrCreateBuilderAndKeepAlive (ctok, options, userOpName)) - use _unwind = decrement + let! builderOpt,creationErrors = execWithReactorAsync (fun ctok -> getOrCreateBuilder (ctok, options, userOpName)) match builderOpt with | None -> return FSharpCheckFileAnswer.Succeeded (MakeCheckFileResultsEmpty(filename, creationErrors)) | Some builder -> @@ -2699,8 +2658,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC Logger.LogMessage (filename + strGuid + "-Cancelling background work") LogCompilerFunctionId.Service_ParseAndCheckFileInProject reactor.CancelBackgroundOp() // cancel the background work, since we will start new work after we're done - let! builderOpt,creationErrors,decrement = execWithReactorAsync (fun ctok -> getOrCreateBuilderAndKeepAlive (ctok, options, userOpName)) - use _unwind = decrement + let! builderOpt,creationErrors = execWithReactorAsync (fun ctok -> getOrCreateBuilder (ctok, options, userOpName)) match builderOpt with | None -> Logger.LogBlockMessageStop (filename + strGuid + "-Failed_Aborted") LogCompilerFunctionId.Service_ParseAndCheckFileInProject @@ -2738,8 +2696,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC member bc.GetBackgroundCheckResultsForFileInProject(filename, options, userOpName) = reactor.EnqueueAndAwaitOpAsync(userOpName, "GetBackgroundCheckResultsForFileInProject", filename, fun ctok -> cancellable { - let! builderOpt, creationErrors, decrement = getOrCreateBuilderAndKeepAlive (ctok, options, userOpName) - use _unwind = decrement + let! builderOpt, creationErrors = getOrCreateBuilder (ctok, options, userOpName) match builderOpt with | None -> let parseResults = FSharpParseFileResults(creationErrors, None, true, [| |]) @@ -2761,7 +2718,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC List.head tcProj.TcResolutionsRev, List.head tcProj.TcSymbolUsesRev, tcProj.TcEnvAtEnd.NameEnv, - loadClosure, reactorOps, (fun () -> builder.IsAlive), None, + loadClosure, reactorOps, None, tcProj.LatestImplementationFile, List.head tcProj.TcOpenDeclarationsRev) let typedResults = MakeCheckFileResults(filename, options, builder, scope, Array.ofList tcProj.TcDependencyFiles, creationErrors, parseResults.Errors, tcErrors) @@ -2782,8 +2739,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC /// Parse and typecheck the whole project (the implementation, called recursively as project graph is evaluated) member private bc.ParseAndCheckProjectImpl(options, ctok, userOpName) : Cancellable = cancellable { - let! builderOpt,creationErrors,decrement = getOrCreateBuilderAndKeepAlive (ctok, options, userOpName) - use _unwind = decrement + let! builderOpt,creationErrors = getOrCreateBuilder (ctok, options, userOpName) match builderOpt with | None -> return FSharpCheckProjectResults (options.ProjectFileName, None, keepAssemblyContents, creationErrors, None) @@ -2803,20 +2759,11 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC // NOTE: This creation of the background builder is currently run as uncancellable. Creating background builders is generally // cheap though the timestamp computations look suspicious for transitive project references. - let builderOpt,_creationErrors,decrement = getOrCreateBuilderAndKeepAlive (ctok, options, userOpName + ".TryGetLogicalTimeStampForProject") |> Cancellable.runWithoutCancellation - use _unwind = decrement + let builderOpt,_creationErrors = getOrCreateBuilder (ctok, options, userOpName + ".TryGetLogicalTimeStampForProject") |> Cancellable.runWithoutCancellation match builderOpt with | None -> None | Some builder -> Some (builder.GetLogicalTimeStampForProject(cache, ctok)) - /// Keep the projet builder alive over a scope - member bc.KeepProjectAlive(options, userOpName) = - reactor.EnqueueAndAwaitOpAsync(userOpName, "KeepProjectAlive", options.ProjectFileName, fun ctok -> - cancellable { - let! _builderOpt,_creationErrors,decrement = getOrCreateBuilderAndKeepAlive (ctok, options, userOpName) - return decrement - }) - /// Parse and typecheck the whole project. member bc.ParseAndCheckProject(options, userOpName) = reactor.EnqueueAndAwaitOpAsync(userOpName, "ParseAndCheckProject", options.ProjectFileName, fun ctok -> bc.ParseAndCheckProjectImpl(options, ctok, userOpName)) @@ -2912,10 +2859,9 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC member bc.CheckProjectInBackground (options, userOpName) = reactor.SetBackgroundOp (Some (userOpName, "CheckProjectInBackground", options.ProjectFileName, (fun ctok ct -> // The creation of the background builder can't currently be cancelled - match getOrCreateBuilderAndKeepAlive (ctok, options, userOpName) |> Cancellable.run ct with + match getOrCreateBuilder (ctok, options, userOpName) |> Cancellable.run ct with | ValueOrCancelled.Cancelled _ -> false - | ValueOrCancelled.Value (builderOpt,_,decrement) -> - use _unwind = decrement + | ValueOrCancelled.Value (builderOpt,_) -> match builderOpt with | None -> false | Some builder -> @@ -3205,10 +3151,6 @@ type FSharpChecker(legacyReferenceResolver, projectCacheSize, keepAssemblyConten ic.CheckMaxMemoryReached() backgroundCompiler.ParseAndCheckProject(options, userOpName) - member ic.KeepProjectAlive(options, ?userOpName: string) = - let userOpName = defaultArg userOpName "Unknown" - backgroundCompiler.KeepProjectAlive(options, userOpName) - /// For a given script file, get the ProjectOptions implied by the #load closure member ic.GetProjectOptionsFromScript(filename, source, ?loadedTimeStamp, ?otherFlags, ?useFsiAuxLib, ?useSdkRefs, ?assumeDotNetFramework, ?extraProjectInfo: obj, ?optionsStamp: int64, ?userOpName: string) = let userOpName = defaultArg userOpName "Unknown" @@ -3325,7 +3267,7 @@ type FsiInteractiveChecker(legacyReferenceResolver, reactorOps: IReactorOperatio CompileOptions.ParseCompilerOptions (ignore, fsiCompilerOptions, [ ]) let loadClosure = LoadClosure.ComputeClosureOfScriptText(ctok, legacyReferenceResolver, defaultFSharpBinariesDir, filename, sourceText, CodeContext.Editing, tcConfig.useSimpleResolution, tcConfig.useFsiAuxLib, tcConfig.useSdkRefs, new Lexhelp.LexResourceManager(), applyCompilerOptions, assumeDotNetFramework, tryGetMetadataSnapshot=(fun _ -> None), reduceMemoryUsage=reduceMemoryUsage) - let! tcErrors, tcFileResult = Parser.CheckOneFile(parseResults, sourceText, filename, "project", tcConfig, tcGlobals, tcImports, tcState, Map.empty, Some loadClosure, backgroundDiagnostics, reactorOps, (fun () -> true), None, userOpName, suggestNamesForErrors) + let! tcErrors, tcFileResult = Parser.CheckOneFile(parseResults, sourceText, filename, "project", tcConfig, tcGlobals, tcImports, tcState, Map.empty, Some loadClosure, backgroundDiagnostics, reactorOps, None, userOpName, suggestNamesForErrors) return match tcFileResult with diff --git a/src/fsharp/service/service.fsi b/src/fsharp/service/service.fsi index a17abb46a3d..ea27c142c12 100755 --- a/src/fsharp/service/service.fsi +++ b/src/fsharp/service/service.fsi @@ -98,6 +98,9 @@ type public FSharpCheckFileResults = /// an unrecoverable error in earlier checking/parsing/resolution steps. member HasFullTypeCheckInfo: bool + /// Tries to get the current successful TcImports. This is only used in testing. Do not use it for other stuff. + member internal TryGetCurrentTcImports: unit -> TcImports option + /// Indicates the set of files which must be watched to accurately track changes that affect these results, /// Clients interested in reacting to updates to these files should watch these files and take actions as described /// in the documentation for compiler service. @@ -504,14 +507,6 @@ type public FSharpChecker = /// An optional string used for tracing compiler operations associated with this request. member ParseAndCheckProject : options: FSharpProjectOptions * ?userOpName: string -> Async - /// - /// Create resources for the project and keep the project alive until the returned object is disposed. - /// - /// - /// The options for the project or script. - /// An optional string used for tracing compiler operations associated with this request. - member KeepProjectAlive : options: FSharpProjectOptions * ?userOpName: string -> Async - /// /// For a given script file, get the FSharpProjectOptions implied by the #load closure. /// All files are read from the FileSystem API, except the file being checked. diff --git a/tests/service/MultiProjectAnalysisTests.fs b/tests/service/MultiProjectAnalysisTests.fs index 45608256535..04ba820da0d 100644 --- a/tests/service/MultiProjectAnalysisTests.fs +++ b/tests/service/MultiProjectAnalysisTests.fs @@ -348,11 +348,6 @@ let ``Test ManyProjectsStressTest cache too small`` () = let checker = ManyProjectsStressTest.makeCheckerForStressTest false - // Because the cache is too small, we need explicit calls to KeepAlive to avoid disposal of project information - let disposals = - [ for p in ManyProjectsStressTest.jointProject :: ManyProjectsStressTest.projects do - yield checker.KeepProjectAlive p.Options |> Async.RunSynchronously ] - let wholeProjectResults = checker.ParseAndCheckProject(ManyProjectsStressTest.jointProject.Options) |> Async.RunSynchronously [ for x in wholeProjectResults.AssemblySignature.Entities -> x.DisplayName ] |> shouldEqual ["JointProject"] diff --git a/tests/service/ProjectAnalysisTests.fs b/tests/service/ProjectAnalysisTests.fs index 08f19cbe708..2af3aab8786 100644 --- a/tests/service/ProjectAnalysisTests.fs +++ b/tests/service/ProjectAnalysisTests.fs @@ -96,7 +96,6 @@ let mmmm2 : M.CAbbrev = new M.CAbbrev() // note, these don't count as uses of C [] let ``Test project1 whole project errors`` () = - let wholeProjectResults = checker.ParseAndCheckProject(Project1.options) |> Async.RunSynchronously wholeProjectResults .Errors.Length |> shouldEqual 2 wholeProjectResults.Errors.[1].Message.Contains("Incomplete pattern matches on this expression") |> shouldEqual true // yes it does @@ -107,6 +106,26 @@ let ``Test project1 whole project errors`` () = wholeProjectResults.Errors.[0].StartColumn |> shouldEqual 43 wholeProjectResults.Errors.[0].EndColumn |> shouldEqual 44 +[] +let ``Test project1 and make sure TcImports gets cleaned up`` () = + + let test () = + let (_, checkFileAnswer) = checker.ParseAndCheckFileInProject(Project1.fileName1, 0, Project1.fileSource1, Project1.options) |> Async.RunSynchronously + match checkFileAnswer with + | FSharpCheckFileAnswer.Aborted -> failwith "should not be aborted" + | FSharpCheckFileAnswer.Succeeded checkFileResults -> + let tcImportsOpt = checkFileResults.TryGetCurrentTcImports () + Assert.True tcImportsOpt.IsSome + let weakTcImports = WeakReference tcImportsOpt.Value + Assert.True weakTcImports.IsAlive + weakTcImports + + let weakTcImports = test () + checker.InvalidateConfiguration (Project1.options) + checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients() + GC.Collect(2, GCCollectionMode.Forced, blocking = true) + Assert.False weakTcImports.IsAlive + [] let ``Test Project1 should have protected FullName and TryFullName return same results`` () = let wholeProjectResults = checker.ParseAndCheckProject(Project1.options) |> Async.RunSynchronously