From 2008c0de97112b3c16686e43522c36948c0a3178 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 21 Oct 2020 13:33:50 -0700 Subject: [PATCH 1/7] Forcing type provider calls on the reactor thread --- src/absil/illib.fs | 9 +++++++++ src/fsharp/CompilerConfig.fs | 12 +++--------- src/fsharp/CompilerConfig.fsi | 8 -------- src/fsharp/CompilerImports.fs | 3 ++- src/fsharp/ExtensionTyping.fs | 5 +++-- src/fsharp/ExtensionTyping.fsi | 3 +++ src/fsharp/service/IncrementalBuild.fs | 6 ++++++ src/fsharp/tainted.fs | 8 ++++---- src/fsharp/tainted.fsi | 3 ++- 9 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/absil/illib.fs b/src/absil/illib.fs index 90709ee6cc7..ed7941678a9 100644 --- a/src/absil/illib.fs +++ b/src/absil/illib.fs @@ -653,6 +653,15 @@ type Lock<'LockTokenType when 'LockTokenType :> LockToken>() = //--------------------------------------------------- // Misc +/// The thread in which compilation calls will be enqueued and done work on. +type internal ICompilationThread = + + /// Enqueue work to be done on a compilation thread. + abstract EnqueueWork: (CompilationThreadToken -> unit) -> unit + + /// Enqueue work to be done on a compilation thread. + abstract EnqueueWorkAndWait: (CompilationThreadToken -> 'T) -> 'T + /// Get an initialization hole let getHole r = match !r with None -> failwith "getHole" | Some x -> x diff --git a/src/fsharp/CompilerConfig.fs b/src/fsharp/CompilerConfig.fs index 060d3f864cc..60898838c5d 100644 --- a/src/fsharp/CompilerConfig.fs +++ b/src/fsharp/CompilerConfig.fs @@ -238,14 +238,6 @@ 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 ImportedAssembly = { ILScopeRef: ILScopeRef FSharpViewOfMetadata: CcuThunk @@ -605,7 +597,9 @@ type TcConfigBuilder = #endif compilationThread = let ctok = CompilationThreadToken () - { new ICompilationThread with member __.EnqueueWork work = work ctok } + { new ICompilationThread with + member _.EnqueueWork work = work ctok + member _.EnqueueWorkAndWait work = work ctok } pause = false alwaysCallVirt = true noDebugData = false diff --git a/src/fsharp/CompilerConfig.fsi b/src/fsharp/CompilerConfig.fsi index 00e824b4054..cb6907d27ac 100644 --- a/src/fsharp/CompilerConfig.fsi +++ b/src/fsharp/CompilerConfig.fsi @@ -92,14 +92,6 @@ type AssemblyReference = type UnresolvedAssemblyReference = UnresolvedAssemblyReference of string * AssemblyReference list -/// 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 diff --git a/src/fsharp/CompilerImports.fs b/src/fsharp/CompilerImports.fs index b54914696de..52c694f76af 100644 --- a/src/fsharp/CompilerImports.fs +++ b/src/fsharp/CompilerImports.fs @@ -1227,7 +1227,8 @@ and [] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse outputFile = tcConfig.outputFile showResolutionMessages = tcConfig.showExtensionTypeMessages referencedAssemblies = Array.distinct [| for r in tcImportsStrong.AllAssemblyResolutions() -> r.resolvedPath |] - temporaryFolder = FileSystem.GetTempPathShim() } + temporaryFolder = FileSystem.GetTempPathShim() + compilationThread = tcConfig.compilationThread } // The type provider should not hold strong references to disposed // TcImport objects. So the callbacks provided in the type provider config diff --git a/src/fsharp/ExtensionTyping.fs b/src/fsharp/ExtensionTyping.fs index 9a064ae17a0..3b244f80c2c 100644 --- a/src/fsharp/ExtensionTyping.fs +++ b/src/fsharp/ExtensionTyping.fs @@ -31,7 +31,8 @@ module internal ExtensionTyping = outputFile : string option showResolutionMessages : bool referencedAssemblies : string[] - temporaryFolder : string } + temporaryFolder : string + compilationThread : ICompilationThread } /// Load a the design-time part of a type-provider into the host process, and look for types /// marked with the TypeProviderAttribute attribute. @@ -97,7 +98,7 @@ module internal ExtensionTyping = // reporting errors. let protect f = try - f () + resolutionEnvironment.compilationThread.EnqueueWorkAndWait(fun _ -> f ()) with err -> let e = StripException (StripException err) raise (TypeProviderError(FSComp.SR.etTypeProviderConstructorException(e.Message), typeProviderImplementationType.FullName, m)) diff --git a/src/fsharp/ExtensionTyping.fsi b/src/fsharp/ExtensionTyping.fsi index 00ffbf1058e..e52669decfc 100755 --- a/src/fsharp/ExtensionTyping.fsi +++ b/src/fsharp/ExtensionTyping.fsi @@ -42,6 +42,9 @@ module internal ExtensionTyping = /// The folder for temporary files temporaryFolder : string + + /// The compilation thread + compilationThread : ICompilationThread } /// Find and instantiate the set of ITypeProvider components for the given assembly reference diff --git a/src/fsharp/service/IncrementalBuild.fs b/src/fsharp/service/IncrementalBuild.fs index ad7ae10b31b..524bf5af7c1 100755 --- a/src/fsharp/service/IncrementalBuild.fs +++ b/src/fsharp/service/IncrementalBuild.fs @@ -2091,6 +2091,12 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput Reactor.Singleton.EnqueueOp ("Unknown", "ICompilationThread.EnqueueWork", "work", fun ctok -> work ctok ) + member __.EnqueueWorkAndWait work = + Reactor.Singleton.EnqueueAndAwaitOpAsync("Unknown", "ICompilationThread.EnqueueWorkAndWait", "work", fun ctok -> + cancellable { + return work ctok + } + ) |> Async.RunSynchronously } tcConfigB, sourceFilesNew diff --git a/src/fsharp/tainted.fs b/src/fsharp/tainted.fs index 15d2378b3fb..c2581184d5d 100644 --- a/src/fsharp/tainted.fs +++ b/src/fsharp/tainted.fs @@ -69,7 +69,7 @@ type internal TypeProviderError for msg in errors do f (new TypeProviderError(errNum, tpDesignation, m, [msg], typeNameContext, methodNameContext)) -type TaintedContext = { TypeProvider : ITypeProvider; TypeProviderAssemblyRef : ILScopeRef } +type TaintedContext = { TypeProvider : ITypeProvider; TypeProviderAssemblyRef : ILScopeRef; CompilationThread: ICompilationThread } [][] type internal Tainted<'T> (context : TaintedContext, value : 'T) = @@ -88,7 +88,7 @@ type internal Tainted<'T> (context : TaintedContext, value : 'T) = member this.Protect f (range:range) = try - f value + context.CompilationThread.EnqueueWorkAndWait (fun _ -> f value) with | :? TypeProviderError -> reraise() | :? AggregateException as ae -> @@ -141,9 +141,9 @@ type internal Tainted<'T> (context : TaintedContext, value : 'T) = /// Access the target object directly. Use with extreme caution. member this.AccessObjectDirectly = value - static member CreateAll(providerSpecs : (ITypeProvider * ILScopeRef) list) = + static member CreateAll(providerSpecs : (ITypeProvider * ILScopeRef) list, compilationThread) = [for (tp,nm) in providerSpecs do - yield Tainted<_>({ TypeProvider=tp; TypeProviderAssemblyRef=nm },tp) ] + yield Tainted<_>({ TypeProvider=tp; TypeProviderAssemblyRef=nm; CompilationThread=compilationThread },tp) ] member this.OfType<'U> () = match box value with diff --git a/src/fsharp/tainted.fsi b/src/fsharp/tainted.fsi index c5220feb6cf..f931380d317 100644 --- a/src/fsharp/tainted.fsi +++ b/src/fsharp/tainted.fsi @@ -10,6 +10,7 @@ open System.Reflection open Microsoft.FSharp.Core.CompilerServices open FSharp.Compiler.Range open FSharp.Compiler.AbstractIL.IL +open FSharp.Compiler.AbstractIL.Internal.Library /// Stores and transports aggregated list of errors reported by the type provider type internal TypeProviderError = @@ -42,7 +43,7 @@ type internal TypeProviderError = type internal Tainted<'T> = /// Create an initial tainted value - static member CreateAll : (ITypeProvider * ILScopeRef) list -> Tainted list + static member CreateAll : (ITypeProvider * ILScopeRef) list * ICompilationThread -> Tainted list /// A type provider that produced the value member TypeProvider : Tainted From cc40667f75335816ec0b3e247eb30e74b5bb9e69 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 21 Oct 2020 14:12:05 -0700 Subject: [PATCH 2/7] Fixing build and preventing deadlock in IncrementalBuild's compilation thread --- src/fsharp/ExtensionTyping.fs | 2 +- src/fsharp/service/IncrementalBuild.fs | 2 +- src/fsharp/service/Reactor.fs | 26 +++++++++++++++++++++++++- src/fsharp/service/Reactor.fsi | 4 ++++ 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/fsharp/ExtensionTyping.fs b/src/fsharp/ExtensionTyping.fs index 3b244f80c2c..a859c760c78 100644 --- a/src/fsharp/ExtensionTyping.fs +++ b/src/fsharp/ExtensionTyping.fs @@ -172,7 +172,7 @@ module internal ExtensionTyping = tpe.Iter(fun e -> errorR(NumberedError((e.Number, e.ContextualErrorMessage), m)) ) [] - let providers = Tainted<_>.CreateAll providerSpecs + let providers = Tainted<_>.CreateAll(providerSpecs, resolutionEnvironment.compilationThread) providers diff --git a/src/fsharp/service/IncrementalBuild.fs b/src/fsharp/service/IncrementalBuild.fs index 524bf5af7c1..88a7b22f33b 100755 --- a/src/fsharp/service/IncrementalBuild.fs +++ b/src/fsharp/service/IncrementalBuild.fs @@ -2092,7 +2092,7 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput work ctok ) member __.EnqueueWorkAndWait work = - Reactor.Singleton.EnqueueAndAwaitOpAsync("Unknown", "ICompilationThread.EnqueueWorkAndWait", "work", fun ctok -> + Reactor.Singleton.ExecuteOrEnqueueAndAwaitOpAsync("Unknown", "ICompilationThread.EnqueueWorkAndWait", "work", fun ctok -> cancellable { return work ctok } diff --git a/src/fsharp/service/Reactor.fs b/src/fsharp/service/Reactor.fs index 5eeb29fe667..dbe9ac55c2a 100755 --- a/src/fsharp/service/Reactor.fs +++ b/src/fsharp/service/Reactor.fs @@ -42,6 +42,9 @@ type Reactor() = let mutable culture = CultureInfo(CultureInfo.CurrentUICulture.Name) let mutable bgOpCts = new CancellationTokenSource() + + let threadsUsed = System.Collections.Concurrent.ConcurrentDictionary() + /// Mailbox dispatch function. let builder = MailboxProcessor<_>.Start <| fun inbox -> @@ -68,7 +71,10 @@ type Reactor() = Trace.TraceInformation("Reactor: {0:n3} pausing {1} milliseconds", DateTime.Now.TimeOfDay.TotalSeconds, pauseBeforeBackgroundWork) pauseBeforeBackgroundWork return! inbox.TryReceive(timeout) } - Thread.CurrentThread.CurrentUICulture <- culture + + let currentThread = Thread.CurrentThread + currentThread.CurrentUICulture <- culture + match msg with | Some (SetBackgroundOp bgOpOpt) -> //Trace.TraceInformation("Reactor: --> set background op, remaining {0}", inbox.CurrentQueueLength) @@ -79,7 +85,12 @@ type Reactor() = Trace.TraceInformation("Reactor: {0:n3} --> {1}.{2} ({3}), remaining {4}", DateTime.Now.TimeOfDay.TotalSeconds, userOpName, opName, opArg, inbox.CurrentQueueLength) let time = Stopwatch() time.Start() + + threadsUsed.[currentThread.ManagedThreadId] <- 0uy op ctok + let res, _ = threadsUsed.TryRemove(currentThread.ManagedThreadId) + assert res + time.Stop() let span = time.Elapsed //if span.TotalMilliseconds > 100.0 then @@ -196,6 +207,19 @@ type Reactor() = ) return! resultCell.AsyncResult } + + member r.ExecuteOrEnqueueAndAwaitOpAsync (userOpName, opName, opArg, f) = + if threadsUsed.ContainsKey(Thread.CurrentThread.ManagedThreadId) then + async { + let! ct = Async.CancellationToken + let result = + match Cancellable.run ct (f (AssumeCompilationThreadWithoutEvidence())) with + | ValueOrCancelled.Value r -> r + | ValueOrCancelled.Cancelled e -> raise e + return result } + else + r.EnqueueAndAwaitOpAsync(userOpName, opName, opArg, f) + member __.PauseBeforeBackgroundWork with get() = pauseBeforeBackgroundWork and set v = pauseBeforeBackgroundWork <- v static member Singleton = theReactor diff --git a/src/fsharp/service/Reactor.fsi b/src/fsharp/service/Reactor.fsi index 3040ca65eee..e01510447cf 100755 --- a/src/fsharp/service/Reactor.fsi +++ b/src/fsharp/service/Reactor.fsi @@ -48,6 +48,10 @@ type internal Reactor = /// Put the operation in the queue, and return an async handle to its result. member EnqueueAndAwaitOpAsync : userOpName:string * opName:string * opArg:string * (CompilationThreadToken -> Cancellable<'T>) -> Async<'T> + /// Put the operation in the queue or run it immediately if the op is trying to be executed on the thread that is currently being used in the reactor. + /// This prevents a deadlock. + member ExecuteOrEnqueueAndAwaitOpAsync : userOpName:string * opName:string * opArg:string * (CompilationThreadToken -> Cancellable<'T>) -> Async<'T> + /// The timespan in milliseconds before background work begins after the operations queue is empty member PauseBeforeBackgroundWork : int with get, set From f085e71b176372bdaea1cf83df6587610bbeb5ab Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 21 Oct 2020 14:19:16 -0700 Subject: [PATCH 3/7] Fixing more deadlocks --- src/fsharp/service/Reactor.fs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/fsharp/service/Reactor.fs b/src/fsharp/service/Reactor.fs index dbe9ac55c2a..4561599ade9 100755 --- a/src/fsharp/service/Reactor.fs +++ b/src/fsharp/service/Reactor.fs @@ -45,6 +45,15 @@ type Reactor() = let threadsUsed = System.Collections.Concurrent.ConcurrentDictionary() + let useThread (thread: Thread) = + thread.CurrentUICulture <- culture + threadsUsed.[thread.ManagedThreadId] <- 0uy + { new IDisposable with + member _.Dispose() = + let res, _ = threadsUsed.TryRemove(thread.ManagedThreadId) + assert res + } + /// Mailbox dispatch function. let builder = MailboxProcessor<_>.Start <| fun inbox -> @@ -72,8 +81,7 @@ type Reactor() = pauseBeforeBackgroundWork return! inbox.TryReceive(timeout) } - let currentThread = Thread.CurrentThread - currentThread.CurrentUICulture <- culture + use _disposable = useThread Thread.CurrentThread match msg with | Some (SetBackgroundOp bgOpOpt) -> @@ -85,12 +93,7 @@ type Reactor() = Trace.TraceInformation("Reactor: {0:n3} --> {1}.{2} ({3}), remaining {4}", DateTime.Now.TimeOfDay.TotalSeconds, userOpName, opName, opArg, inbox.CurrentQueueLength) let time = Stopwatch() time.Start() - - threadsUsed.[currentThread.ManagedThreadId] <- 0uy op ctok - let res, _ = threadsUsed.TryRemove(currentThread.ManagedThreadId) - assert res - time.Stop() let span = time.Elapsed //if span.TotalMilliseconds > 100.0 then From ffcbdea4eb50a5f79fee15530b84a37db25e65b0 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Thu, 22 Oct 2020 10:04:08 -0700 Subject: [PATCH 4/7] Removed ICompilationThread and added TypeProviderLock singleton --- src/absil/illib.fs | 9 --------- src/fsharp/CompilerConfig.fs | 7 ------- src/fsharp/CompilerConfig.fsi | 2 -- src/fsharp/CompilerImports.fs | 24 ++++++++++++------------ src/fsharp/ExtensionTyping.fs | 7 +++---- src/fsharp/ExtensionTyping.fsi | 3 --- src/fsharp/service/IncrementalBuild.fs | 14 -------------- src/fsharp/tainted.fs | 17 +++++++++++++---- src/fsharp/tainted.fsi | 12 +++++++++++- 9 files changed, 39 insertions(+), 56 deletions(-) diff --git a/src/absil/illib.fs b/src/absil/illib.fs index ed7941678a9..90709ee6cc7 100644 --- a/src/absil/illib.fs +++ b/src/absil/illib.fs @@ -653,15 +653,6 @@ type Lock<'LockTokenType when 'LockTokenType :> LockToken>() = //--------------------------------------------------- // Misc -/// The thread in which compilation calls will be enqueued and done work on. -type internal ICompilationThread = - - /// Enqueue work to be done on a compilation thread. - abstract EnqueueWork: (CompilationThreadToken -> unit) -> unit - - /// Enqueue work to be done on a compilation thread. - abstract EnqueueWorkAndWait: (CompilationThreadToken -> 'T) -> 'T - /// Get an initialization hole let getHole r = match !r with None -> failwith "getHole" | Some x -> x diff --git a/src/fsharp/CompilerConfig.fs b/src/fsharp/CompilerConfig.fs index 60898838c5d..3fe67f47a53 100644 --- a/src/fsharp/CompilerConfig.fs +++ b/src/fsharp/CompilerConfig.fs @@ -434,7 +434,6 @@ type TcConfigBuilder = /// show messages about extension type resolution? mutable showExtensionTypeMessages: bool #endif - mutable compilationThread: ICompilationThread /// pause between passes? mutable pause: bool @@ -595,11 +594,6 @@ type TcConfigBuilder = #if !NO_EXTENSIONTYPING showExtensionTypeMessages = false #endif - compilationThread = - let ctok = CompilationThreadToken () - { new ICompilationThread with - member _.EnqueueWork work = work ctok - member _.EnqueueWorkAndWait work = work ctok } pause = false alwaysCallVirt = true noDebugData = false @@ -994,7 +988,6 @@ type TcConfig private (data: TcConfigBuilder, validate: bool) = #if !NO_EXTENSIONTYPING 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 diff --git a/src/fsharp/CompilerConfig.fsi b/src/fsharp/CompilerConfig.fsi index cb6907d27ac..a7f2d9d70df 100644 --- a/src/fsharp/CompilerConfig.fsi +++ b/src/fsharp/CompilerConfig.fsi @@ -243,7 +243,6 @@ type TcConfigBuilder = #if !NO_EXTENSIONTYPING mutable showExtensionTypeMessages: bool #endif - mutable compilationThread: ICompilationThread mutable pause: bool mutable alwaysCallVirt: bool mutable noDebugData: bool @@ -421,7 +420,6 @@ type TcConfig = #if !NO_EXTENSIONTYPING member showExtensionTypeMessages: bool #endif - member compilationThread: ICompilationThread member pause: bool member alwaysCallVirt: bool member noDebugData: bool diff --git a/src/fsharp/CompilerImports.fs b/src/fsharp/CompilerImports.fs index 52c694f76af..1664b0dedba 100644 --- a/src/fsharp/CompilerImports.fs +++ b/src/fsharp/CompilerImports.fs @@ -695,9 +695,9 @@ type RawFSharpAssemblyDataBackedByFileOnDisk (ilModule: ILModuleDef, ilAssemblyR type TcImportsSafeDisposal (disposeActions: ResizeArray unit>, #if !NO_EXTENSIONTYPING - disposeTypeProviderActions: ResizeArray unit>, + disposeTypeProviderActions: ResizeArray unit> #endif - compilationThread: ICompilationThread) = + ) = let mutable isDisposed = false @@ -707,9 +707,11 @@ type TcImportsSafeDisposal if verbose then dprintf "disposing of TcImports, %d binaries\n" disposeActions.Count #if !NO_EXTENSIONTYPING - let actions = disposeTypeProviderActions - if actions.Count > 0 then - compilationThread.EnqueueWork (fun _ -> for action in actions do action()) + async { + let actions = disposeTypeProviderActions + if actions.Count > 0 then + TypeProviderLock.Singleton.AcquireLock(fun _ -> for action in actions do action()) + } |> Async.Start // Make this async so we do not block dispose #endif for action in disposeActions do action() @@ -760,8 +762,7 @@ and TcImportsWeakHack (tcImports: WeakReference) = /// 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, - dependencyProviderOpt: DependencyProvider option) as this = + ilGlobalsOpt, dependencyProviderOpt: DependencyProvider option) as this = let mutable resolutions = initialResolutions let mutable importsBase: TcImports option = importsBase @@ -786,7 +787,7 @@ and [] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse let mutable tcImportsWeak = TcImportsWeakHack (WeakReference<_> this) #endif - let disposal = new TcImportsSafeDisposal(disposeActions, disposeTypeProviderActions, compilationThread) + let disposal = new TcImportsSafeDisposal(disposeActions, disposeTypeProviderActions) let CheckDisposed() = if disposed then assert false @@ -1227,8 +1228,7 @@ and [] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse outputFile = tcConfig.outputFile showResolutionMessages = tcConfig.showExtensionTypeMessages referencedAssemblies = Array.distinct [| for r in tcImportsStrong.AllAssemblyResolutions() -> r.resolvedPath |] - temporaryFolder = FileSystem.GetTempPathShim() - compilationThread = tcConfig.compilationThread } + temporaryFolder = FileSystem.GetTempPathShim() } // The type provider should not hold strong references to disposed // TcImport objects. So the callbacks provided in the type provider config @@ -1668,7 +1668,7 @@ and [] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse let tcResolutions = TcAssemblyResolutions.BuildFromPriorResolutions(ctok, tcConfig, frameworkDLLs, []) let tcAltResolutions = TcAssemblyResolutions.BuildFromPriorResolutions(ctok, tcConfig, nonFrameworkDLLs, []) - let frameworkTcImports = new TcImports(tcConfigP, tcResolutions, None, None, tcConfig.compilationThread, None) + let frameworkTcImports = new TcImports(tcConfigP, tcResolutions, None, None, None) // Fetch the primaryAssembly from the referenced assemblies otherwise let primaryAssemblyReference = @@ -1802,7 +1802,7 @@ and [] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse 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, tcConfig.compilationThread, Some dependencyProvider) + let tcImports = new TcImports(tcConfigP, tcResolutions, Some baseTcImports, Some tcGlobals.ilg, Some dependencyProvider) let! _assemblies = tcImports.RegisterAndImportReferencedAssemblies(ctok, references) tcImports.ReportUnresolvedAssemblyReferences knownUnresolved return tcImports diff --git a/src/fsharp/ExtensionTyping.fs b/src/fsharp/ExtensionTyping.fs index a859c760c78..26be043d15d 100644 --- a/src/fsharp/ExtensionTyping.fs +++ b/src/fsharp/ExtensionTyping.fs @@ -31,8 +31,7 @@ module internal ExtensionTyping = outputFile : string option showResolutionMessages : bool referencedAssemblies : string[] - temporaryFolder : string - compilationThread : ICompilationThread } + temporaryFolder : string } /// Load a the design-time part of a type-provider into the host process, and look for types /// marked with the TypeProviderAttribute attribute. @@ -98,7 +97,7 @@ module internal ExtensionTyping = // reporting errors. let protect f = try - resolutionEnvironment.compilationThread.EnqueueWorkAndWait(fun _ -> f ()) + TypeProviderLock.Singleton.AcquireLock(fun _ -> f ()) with err -> let e = StripException (StripException err) raise (TypeProviderError(FSComp.SR.etTypeProviderConstructorException(e.Message), typeProviderImplementationType.FullName, m)) @@ -172,7 +171,7 @@ module internal ExtensionTyping = tpe.Iter(fun e -> errorR(NumberedError((e.Number, e.ContextualErrorMessage), m)) ) [] - let providers = Tainted<_>.CreateAll(providerSpecs, resolutionEnvironment.compilationThread) + let providers = Tainted<_>.CreateAll(providerSpecs) providers diff --git a/src/fsharp/ExtensionTyping.fsi b/src/fsharp/ExtensionTyping.fsi index e52669decfc..00ffbf1058e 100755 --- a/src/fsharp/ExtensionTyping.fsi +++ b/src/fsharp/ExtensionTyping.fsi @@ -42,9 +42,6 @@ module internal ExtensionTyping = /// The folder for temporary files temporaryFolder : string - - /// The compilation thread - compilationThread : ICompilationThread } /// Find and instantiate the set of ITypeProvider components for the given assembly reference diff --git a/src/fsharp/service/IncrementalBuild.fs b/src/fsharp/service/IncrementalBuild.fs index 88a7b22f33b..68020be8df4 100755 --- a/src/fsharp/service/IncrementalBuild.fs +++ b/src/fsharp/service/IncrementalBuild.fs @@ -2085,20 +2085,6 @@ 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 - ) - member __.EnqueueWorkAndWait work = - Reactor.Singleton.ExecuteOrEnqueueAndAwaitOpAsync("Unknown", "ICompilationThread.EnqueueWorkAndWait", "work", fun ctok -> - cancellable { - return work ctok - } - ) |> Async.RunSynchronously - } - tcConfigB, sourceFilesNew match loadClosureOpt with diff --git a/src/fsharp/tainted.fs b/src/fsharp/tainted.fs index c2581184d5d..5b7b94f3c1b 100644 --- a/src/fsharp/tainted.fs +++ b/src/fsharp/tainted.fs @@ -10,6 +10,15 @@ open Microsoft.FSharp.Core.CompilerServices open FSharp.Compiler.AbstractIL.IL open FSharp.Compiler.AbstractIL.Internal.Library +[] +type internal TypeProviderToken() = interface LockToken + +[] +type internal TypeProviderLock() = + inherit Lock() + + static member val Singleton = TypeProviderLock() + type internal TypeProviderError ( errNum : int, @@ -69,7 +78,7 @@ type internal TypeProviderError for msg in errors do f (new TypeProviderError(errNum, tpDesignation, m, [msg], typeNameContext, methodNameContext)) -type TaintedContext = { TypeProvider : ITypeProvider; TypeProviderAssemblyRef : ILScopeRef; CompilationThread: ICompilationThread } +type TaintedContext = { TypeProvider : ITypeProvider; TypeProviderAssemblyRef : ILScopeRef } [][] type internal Tainted<'T> (context : TaintedContext, value : 'T) = @@ -88,7 +97,7 @@ type internal Tainted<'T> (context : TaintedContext, value : 'T) = member this.Protect f (range:range) = try - context.CompilationThread.EnqueueWorkAndWait (fun _ -> f value) + TypeProviderLock.Singleton.AcquireLock(fun _ -> f value) with | :? TypeProviderError -> reraise() | :? AggregateException as ae -> @@ -141,9 +150,9 @@ type internal Tainted<'T> (context : TaintedContext, value : 'T) = /// Access the target object directly. Use with extreme caution. member this.AccessObjectDirectly = value - static member CreateAll(providerSpecs : (ITypeProvider * ILScopeRef) list, compilationThread) = + static member CreateAll(providerSpecs : (ITypeProvider * ILScopeRef) list) = [for (tp,nm) in providerSpecs do - yield Tainted<_>({ TypeProvider=tp; TypeProviderAssemblyRef=nm; CompilationThread=compilationThread },tp) ] + yield Tainted<_>({ TypeProvider=tp; TypeProviderAssemblyRef=nm },tp) ] member this.OfType<'U> () = match box value with diff --git a/src/fsharp/tainted.fsi b/src/fsharp/tainted.fsi index f931380d317..45c9f9f572f 100644 --- a/src/fsharp/tainted.fsi +++ b/src/fsharp/tainted.fsi @@ -12,6 +12,16 @@ open FSharp.Compiler.Range open FSharp.Compiler.AbstractIL.IL open FSharp.Compiler.AbstractIL.Internal.Library +[] +type internal TypeProviderToken = + interface LockToken + +[] +type internal TypeProviderLock = + inherit Lock + + static member Singleton : TypeProviderLock + /// Stores and transports aggregated list of errors reported by the type provider type internal TypeProviderError = inherit System.Exception @@ -43,7 +53,7 @@ type internal TypeProviderError = type internal Tainted<'T> = /// Create an initial tainted value - static member CreateAll : (ITypeProvider * ILScopeRef) list * ICompilationThread -> Tainted list + static member CreateAll : (ITypeProvider * ILScopeRef) list -> Tainted list /// A type provider that produced the value member TypeProvider : Tainted From 91af6e8d92f4485fee1def35d55dee69e378806c Mon Sep 17 00:00:00 2001 From: Will Smith Date: Thu, 22 Oct 2020 10:12:18 -0700 Subject: [PATCH 5/7] Removing reactor changes --- src/fsharp/service/Reactor.fs | 27 +-------------------------- src/fsharp/service/Reactor.fsi | 4 ---- 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/src/fsharp/service/Reactor.fs b/src/fsharp/service/Reactor.fs index 4561599ade9..29cf0c9678b 100755 --- a/src/fsharp/service/Reactor.fs +++ b/src/fsharp/service/Reactor.fs @@ -43,17 +43,6 @@ type Reactor() = let mutable bgOpCts = new CancellationTokenSource() - let threadsUsed = System.Collections.Concurrent.ConcurrentDictionary() - - let useThread (thread: Thread) = - thread.CurrentUICulture <- culture - threadsUsed.[thread.ManagedThreadId] <- 0uy - { new IDisposable with - member _.Dispose() = - let res, _ = threadsUsed.TryRemove(thread.ManagedThreadId) - assert res - } - /// Mailbox dispatch function. let builder = MailboxProcessor<_>.Start <| fun inbox -> @@ -80,9 +69,7 @@ type Reactor() = Trace.TraceInformation("Reactor: {0:n3} pausing {1} milliseconds", DateTime.Now.TimeOfDay.TotalSeconds, pauseBeforeBackgroundWork) pauseBeforeBackgroundWork return! inbox.TryReceive(timeout) } - - use _disposable = useThread Thread.CurrentThread - + Thread.CurrentThread.CurrentUICulture <- culture match msg with | Some (SetBackgroundOp bgOpOpt) -> //Trace.TraceInformation("Reactor: --> set background op, remaining {0}", inbox.CurrentQueueLength) @@ -211,18 +198,6 @@ type Reactor() = return! resultCell.AsyncResult } - member r.ExecuteOrEnqueueAndAwaitOpAsync (userOpName, opName, opArg, f) = - if threadsUsed.ContainsKey(Thread.CurrentThread.ManagedThreadId) then - async { - let! ct = Async.CancellationToken - let result = - match Cancellable.run ct (f (AssumeCompilationThreadWithoutEvidence())) with - | ValueOrCancelled.Value r -> r - | ValueOrCancelled.Cancelled e -> raise e - return result } - else - r.EnqueueAndAwaitOpAsync(userOpName, opName, opArg, f) - member __.PauseBeforeBackgroundWork with get() = pauseBeforeBackgroundWork and set v = pauseBeforeBackgroundWork <- v static member Singleton = theReactor diff --git a/src/fsharp/service/Reactor.fsi b/src/fsharp/service/Reactor.fsi index e01510447cf..3040ca65eee 100755 --- a/src/fsharp/service/Reactor.fsi +++ b/src/fsharp/service/Reactor.fsi @@ -48,10 +48,6 @@ type internal Reactor = /// Put the operation in the queue, and return an async handle to its result. member EnqueueAndAwaitOpAsync : userOpName:string * opName:string * opArg:string * (CompilationThreadToken -> Cancellable<'T>) -> Async<'T> - /// Put the operation in the queue or run it immediately if the op is trying to be executed on the thread that is currently being used in the reactor. - /// This prevents a deadlock. - member ExecuteOrEnqueueAndAwaitOpAsync : userOpName:string * opName:string * opArg:string * (CompilationThreadToken -> Cancellable<'T>) -> Async<'T> - /// The timespan in milliseconds before background work begins after the operations queue is empty member PauseBeforeBackgroundWork : int with get, set From afbe8e0776c0e03ae2118bd121e2ad82efc17b03 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Thu, 22 Oct 2020 13:03:49 -0700 Subject: [PATCH 6/7] Update CompilerImports.fs --- src/fsharp/CompilerImports.fs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/fsharp/CompilerImports.fs b/src/fsharp/CompilerImports.fs index 1664b0dedba..d7bf6cd1af2 100644 --- a/src/fsharp/CompilerImports.fs +++ b/src/fsharp/CompilerImports.fs @@ -707,11 +707,9 @@ type TcImportsSafeDisposal if verbose then dprintf "disposing of TcImports, %d binaries\n" disposeActions.Count #if !NO_EXTENSIONTYPING - async { - let actions = disposeTypeProviderActions - if actions.Count > 0 then - TypeProviderLock.Singleton.AcquireLock(fun _ -> for action in actions do action()) - } |> Async.Start // Make this async so we do not block dispose + let actions = disposeTypeProviderActions + if actions.Count > 0 then + TypeProviderLock.Singleton.AcquireLock(fun _ -> for action in actions do action()) #endif for action in disposeActions do action() From 08c32bc9d7b908abd98e0c124e00ca3bbf1bfb78 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Fri, 23 Oct 2020 13:40:13 -0700 Subject: [PATCH 7/7] Lock per TP instance --- src/fsharp/CompilerImports.fs | 8 ++------ src/fsharp/ExtensionTyping.fs | 2 +- src/fsharp/tainted.fs | 8 +++----- src/fsharp/tainted.fsi | 2 -- 4 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/fsharp/CompilerImports.fs b/src/fsharp/CompilerImports.fs index 1664b0dedba..ea9fcc7c2c0 100644 --- a/src/fsharp/CompilerImports.fs +++ b/src/fsharp/CompilerImports.fs @@ -707,11 +707,7 @@ type TcImportsSafeDisposal if verbose then dprintf "disposing of TcImports, %d binaries\n" disposeActions.Count #if !NO_EXTENSIONTYPING - async { - let actions = disposeTypeProviderActions - if actions.Count > 0 then - TypeProviderLock.Singleton.AcquireLock(fun _ -> for action in actions do action()) - } |> Async.Start // Make this async so we do not block dispose + for action in disposeTypeProviderActions do action() #endif for action in disposeActions do action() @@ -1258,7 +1254,7 @@ and [] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse for provider in providers do tcImportsStrong.AttachDisposeTypeProviderAction(fun () -> try - provider.PUntaintNoFailure(fun x -> x).Dispose() + provider.PUntaintNoFailure(fun x -> x.Dispose()) with e -> ()) diff --git a/src/fsharp/ExtensionTyping.fs b/src/fsharp/ExtensionTyping.fs index 26be043d15d..215612e8e09 100644 --- a/src/fsharp/ExtensionTyping.fs +++ b/src/fsharp/ExtensionTyping.fs @@ -97,7 +97,7 @@ module internal ExtensionTyping = // reporting errors. let protect f = try - TypeProviderLock.Singleton.AcquireLock(fun _ -> f ()) + f () with err -> let e = StripException (StripException err) raise (TypeProviderError(FSComp.SR.etTypeProviderConstructorException(e.Message), typeProviderImplementationType.FullName, m)) diff --git a/src/fsharp/tainted.fs b/src/fsharp/tainted.fs index 5b7b94f3c1b..be352384b57 100644 --- a/src/fsharp/tainted.fs +++ b/src/fsharp/tainted.fs @@ -17,8 +17,6 @@ type internal TypeProviderToken() = interface LockToken type internal TypeProviderLock() = inherit Lock() - static member val Singleton = TypeProviderLock() - type internal TypeProviderError ( errNum : int, @@ -78,7 +76,7 @@ type internal TypeProviderError for msg in errors do f (new TypeProviderError(errNum, tpDesignation, m, [msg], typeNameContext, methodNameContext)) -type TaintedContext = { TypeProvider : ITypeProvider; TypeProviderAssemblyRef : ILScopeRef } +type TaintedContext = { TypeProvider : ITypeProvider; TypeProviderAssemblyRef : ILScopeRef; Lock: TypeProviderLock } [][] type internal Tainted<'T> (context : TaintedContext, value : 'T) = @@ -97,7 +95,7 @@ type internal Tainted<'T> (context : TaintedContext, value : 'T) = member this.Protect f (range:range) = try - TypeProviderLock.Singleton.AcquireLock(fun _ -> f value) + context.Lock.AcquireLock(fun _ -> f value) with | :? TypeProviderError -> reraise() | :? AggregateException as ae -> @@ -152,7 +150,7 @@ type internal Tainted<'T> (context : TaintedContext, value : 'T) = static member CreateAll(providerSpecs : (ITypeProvider * ILScopeRef) list) = [for (tp,nm) in providerSpecs do - yield Tainted<_>({ TypeProvider=tp; TypeProviderAssemblyRef=nm },tp) ] + yield Tainted<_>({ TypeProvider=tp; TypeProviderAssemblyRef=nm; Lock=TypeProviderLock() },tp) ] member this.OfType<'U> () = match box value with diff --git a/src/fsharp/tainted.fsi b/src/fsharp/tainted.fsi index 45c9f9f572f..9b1e8cb2198 100644 --- a/src/fsharp/tainted.fsi +++ b/src/fsharp/tainted.fsi @@ -20,8 +20,6 @@ type internal TypeProviderToken = type internal TypeProviderLock = inherit Lock - static member Singleton : TypeProviderLock - /// Stores and transports aggregated list of errors reported by the type provider type internal TypeProviderError = inherit System.Exception