Audit RequireCompilationThread and related not enforced phantom token/lock invariants wrt parallel compilation #20804

Activity

1. majocha

   added

   Needs-Triage

   on Oct 18, 2026
2. github-project-automation

   added this to F# Compiler and Toolingon Oct 18, 2026
3. github-project-automation

   moved this to New in F# Compiler and Toolingon Oct 18, 2026
4. majocha

   changed the title [-]Audit `CompilationThreadToken` and related not enforced phantom token/lock types wrt parallel compilation[/-] [+]Audit `RequireCompilationThread` and related not enforced phantom token/lock invariants wrt parallel compilation[/+] on Oct 18, 2026
5. majocha commented on Oct 18, 2026

   

   majocha

   on Oct 18, 2026

   ContributorAuthor

   More actions

   A quick Opus 5.5 run gives:

   I found two likely concurrency bugs and several lower-risk races. All the token types are phantom (Unchecked.defaultof): they record intent but nothing enforces them at runtime. Some were never checked against parallel execution. I didn't change any code.

   Likely bugs

   1. Two threads can load the same assembly twice (CompilerImports.fs). FindCcuInfo and TryFindDllInfo look the assembly up, call ImplicitLoadIfAllowed, then look it up again. Nothing holds a lock across those three steps. registerDll has the same gap between its IsAlreadyRegistered check and RegisterDll.

      • This is reachable from graph-parallel type checking: import.fs fabricates a ctok and calls FindCcuFromAssemblyRef(lookupOnly=false).
      • It needs implicitlyResolveAssemblies, which is on by default for command-line fsc without --noframework, and for fsi.
      • The likely result is duplicate RegisterDll/RegisterCcu entries and two CcuThunks for one assembly, which could cause type-identity mismatches.
      • The load path also blocks a thread-pool worker with Async.RunSynchronously.

   2. Possible deadlock in fixupOrphanCcus. It runs fixup closures while holding tciLock. Those closures call FindCcu(lookupOnly=false), which can trigger an implicit load. With ParallelReferenceResolution.On (the default), that load waits on worker threads that also need tciLock (RegisterDll, RegisterCcu), so it can deadlock.

   Other races, roughly by severity

   3. Shared state in parallel reference resolution. With ParallelReferenceResolution.On, phase2 runs concurrently and passes the same ctok to every worker. Type-provider instantiation (ImportTypeProviderExtensions) is guarded only by RequireCompilationThread ctok, which does nothing. The calls themselves are serialized by the per-provider TypeProviderLock. However, InjectProvidedNamespaceOrTypeIntoEntity changes the shared entityToInjectInto without that protection.

   4. fsi's AssemblyResolve handler. It fabricates a ctok and runs on whichever thread is loading an assembly, including background threads started by user scripts. FindDynamicAssembly iterates dynamicAssemblies (a ResizeArray) while the fsi thread may be calling .Add on it. On those threads, diagnostics fall back to AssertFalseDiagnosticsLogger and are silently dropped.

   5. Graph-checking finishers. Finishers are re-applied once per dependent node, on arbitrary threads, outside the per-file logger scope. For example, the buildImplementationAlreadyGiven errorR call in the impl finisher writes concurrently to a shared logger. That logger is often a CapturingDiagnosticsLogger, which uses an unsynchronized ResizeArray and errorCount++. There is an existing TODO about this at ParseAndCheckInputs.fs:1600.

   6. MultipleDiagnosticsLoggers.Parallel. It starts replayDiagnostics, which reads the ambient logger, when the function is called rather than when the async runs. If the async runs later or in another context, diagnostics go to the wrong logger. The replay also writes to that logger from a thread-pool thread.

   7. Forged lock tokens. ClearAllILModuleReaderCache (ilread.fs:5166) clears the cache by constructing ILModuleReaderCache1LockToken() directly instead of taking ilModuleReaderCache1Lock.

   8. Caches with no lock at all. AgedLookup rewrites its internal list even on TryGet, so concurrent use loses updates or clears (no structural corruption).

      • braceMatchCache (service.fs) is commented "safe for concurrent access" but has no lock.
      • getToolTipTextCache (FSharpCheckerResults.fs) can be hit concurrently by the IDE.
      • In BackgroundCompiler, InvalidateConfiguration and NotifyProjectCleaned call ContainsSimilarKey outside gate, so the check and the later rebuild can interleave with other calls.

   Stale or misleading markers

   • IncrementalBuild.fs:510 says the cache is "protected via CompilationThreadToken", but it actually uses AnyCallerThreadToken plus gate.
   • import.fs, QuotationTranslator.fs and CheckDeclarations.fs justify AssumeCompilationThreadWithoutEvidence with "we are on the compilation thread", which no longer holds.
   • ParseAndCheckInputs.fs has ignore ctok // TODO Use it.
   • DoesNotRequireCompilerThreadTokenAndCouldPossiblyBeMadeConcurrent is defined but never used.
   • FxResolver.executeProcess locks on obj (the constructor function value) rather than obj (). It works, but probably wasn't intended.
   • TryResolveAssemblyReference holds tciLock for the whole MSBuild resolution, which serializes those calls.

   👍React with 👍1Reacted by Tomas Grosup
6. github-actions

   added this to the Backlog milestone on Oct 18, 2026
7. T-Gro

   removed

   Needs-Triage

   on Oct 19, 2026