Non-cancellable metadata access when importing a DLL

[auduchinok]

Another case of non-cancellable metadata access:

JetBrains.Diagnostics.Assertion+AssertionException
   at JetBrains.Diagnostics.Assertion.Fail(String message)
   at JetBrains.Diagnostics.Assertion.Assert(Boolean condition, String message)
   at JetBrains.ReSharper.Plugins.FSharp.FSharpAsyncUtil.CheckAndThrow(IShellLocks locks) in C:\Developer\resharper-fsharp\ReSharper.FSharp\src\FSharp\FSharp.ProjectModelBase\src\FSharpAsyncUtil.cs:line 33
   at JetBrains.ReSharper.Plugins.FSharp.Shim.AssemblyReader.ProjectFcsModuleReader.JetBrains.ReSharper.Plugins.FSharp.Shim.AssemblyReader.IProjectFcsModuleReader.get_Timestamp() in C:\Developer\resharper-fsharp\ReSharper.FSharp\src\FSharp\FSharp.Common\src\Shim\AssemblyReader\ProjectFcsModuleReader.fs:line 1427
   at <StartupCode$JetBrains-ReSharper-Plugins-FSharp-Common>[email protected](Unit unitVar0) in C:\Developer\resharper-fsharp\ReSharper.FSharp\src\FSharp\FSharp.Common\src\Checker\FcsProjectProvider.fs:line 263
   at <StartupCode$FSharp-Compiler-Service>.$BackgroundCompiler.clo@353-563.FSharp.Compiler.CompilerConfig.IProjectReference.TryGetLogicalTimeStamp(TimeStampCache _arg6) in C:\Developer\jetbrains-fcs\src\Compiler\Service\BackgroundCompiler.fs:line 363
   at FSharp.Compiler.CompilerConfig.TimeStampCache.GetProjectReferenceTimeStamp(IProjectReference projectReference) in C:\Developer\jetbrains-fcs\src\Compiler\Driver\CompilerConfig.fs:line 286
   at <StartupCode$FSharp-Compiler-Service>[email protected](TimeStampCache cache) in C:\Developer\jetbrains-fcs\src\Compiler\Service\IncrementalBuild.fs:line 1547
   at FSharp.Compiler.CodeAnalysis.IncrementalBuilderStateHelpers.computeStampedReferencedAssemblies@1036.Invoke(Int32 i, Tuple`2 asmInfo) in C:\Developer\jetbrains-fcs\src\Compiler\Service\IncrementalBuild.fs:line 1039
   at Internal.Utilities.Library.Block.ImmutableArray.iteri[T](FSharpFunc`2 f, ImmutableArray`1 arr) in C:\Developer\jetbrains-fcs\src\Compiler\Utilities\ImmutableArray.fs:line 37
   at FSharp.Compiler.CodeAnalysis.IncrementalBuilderStateHelpers.computeStampedReferencedAssemblies(IncrementalBuilderInitialState initialState, IncrementalBuilderState state, Boolean canTriggerInvalidation, TimeStampCache cache) in C:\Developer\jetbrains-fcs\src\Compiler\Service\IncrementalBuild.fs:line 1035
   at FSharp.Compiler.CodeAnalysis.IncrementalBuilder.get_IsReferencesInvalidated() in C:\Developer\jetbrains-fcs\src\Compiler\Service\IncrementalBuild.fs:line 1214
   at <StartupCode$FSharp-Compiler-Service>[email protected](Tuple`2 _arg10) in C:\Developer\jetbrains-fcs\src\Compiler\Service\BackgroundCompiler.fs:line 509
   at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvokeNoHijackCheck[a,b](AsyncActivation`1 ctxt, b result1, FSharpFunc`2 userCode) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 528
   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 112

Relates to #18235.

@majocha Would you be interested to take a look? 🙂

[auduchinok]

I guess a fix could be to make CompilerConfig.IProjectReference.TryGetLogicalTimeStamp async, like EvaluateRawContents?

[majocha]

Yes, the problem is the call is not made from within cancellable but directly from async code here:

fsharp/src/Compiler/Service/BackgroundCompiler.fs

Lines 502 to 507 in 2edab12

	let getOrCreateBuilder (options, userOpName) : Async<IncrementalBuilder option * FSharpDiagnostic[]> =
	match tryGetBuilder options with
	| Some getBuilder ->
	async {
	match! getBuilder with
	| builderOpt, creationDiags when builderOpt.IsNone || not builderOpt.Value.IsReferencesInvalidated ->

Solution would be as before, but I really don't like such ad hoc fixes.
It would be best to have a dedicated CE to do this for us. This probably means adjusting cancellableTask for the purpose and replacing all usages of async throughout. Performance and debugging would benefit too, probably.

[majocha]

I guess a fix could be to make CompilerConfig.IProjectReference.TryGetLogicalTimeStamp async, like EvaluateRawContents?

The problem is, currently Cancellable automatically captures the token in toAsync. The problem is sometimes we don't drop from async into cancellable.

[auduchinok]

Solution would be as before, but I really don't like such ad hoc fixes.

Thanks for the pointer, I've done it like this in #18283 for now.