AsyncMemoize: do not cancel and restart jobs that are being requested

[majocha]

This utilizes changes from #16533

The idea is to never cancel a job that is awaited by someone. Basically cancelling a Get does not cancel the cached computation if there are other requests so there is no need for restarts. I removed Linked CTS so computations can only be canceled by posting a message to processStateUpdate.

Disclaimer: I'm probably missing something and don't understand how this is expected to work, but just wanted to see it this passes, because it's green locally.

[github-actions[bot]]

:heavy_exclamation_mark: Release notes required

---

:white_check_mark: Found changes and release notes in following paths:

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/8.0.300.md

[majocha]

Wow, so MacOS and Linux passes 😅.

The one failure seems unrelated, and due to concurrent access to StringBuilder in test utils:

Error message
System.ArgumentOutOfRangeException : Index was out of range. Must be non-negative and less than or equal to the size of the collection. (Parameter 'chunkLength')



Stack trace
   at System.Text.StringBuilder.ToString()
   at FSharp.Test.Utilities.RedirectConsole.Output() in D:\a\_work\1\s\tests\FSharp.Test.Utilities\Utilities.fs:line 114
   at FSharp.Test.Compiler.compileFSharpCompilation(Compilation compilation, Boolean ignoreWarnings, CompilationUnit cUnit) in D:\a\_work\1\s\tests\FSharp.Test.Utilities\Compiler.fs:line 684
   at FSharp.Test.Compiler.compileFSharp(FSharpCompilationSource fs) in D:\a\_work\1\s\tests\FSharp.Test.Utilities\Compiler.fs:line 707
   at FSharp.Test.Compiler.compile(CompilationUnit cUnit) in D:\a\_work\1\s\tests\FSharp.Test.Utilities\Compiler.fs:line 793
   at FSharp.Compiler.UnitTests.AssemblySigning.AssemblyKeyNameAttribute NETCOREAPP() in D:\a\_work\1\s\tests\FSharp.Compiler.UnitTests\AssemblySigningAttributes.fs:line 65
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

We should probably add some locks there.

[majocha]

@0101, if this is checks green, please take a look if this is the right direction.

[0101]

I'm not sure I get the logic of cancelling and restarting jobs. I mean why can one requestor cancel a job that has other active requestors?

The idea behind this is that the computation will run directly in the first requestor's thread and therefore will retain the stack trace. Which would be more valuable than saving the occasional restarts.

Unfortunately as it is now, the computation still gets disconnected. However it's still my ambition to make the stack traces work.

It might be nice to have this configurable and have stack traces for debugging and no-restarts for release.

[majocha]

It might be nice to have this configurable and have stack traces for debugging and no-restarts for release.

Debugging as in debugging the FCS itself?

[0101]

Debugging as in debugging the FCS itself?

Yep. Sometimes it would be nice to know when an exception happens what was the whole path to it. Now you just get the job where it happened being fired off form the thread pool.

[majocha]

Hmmm ok, so this passes without reverting16348 (#16536).

[0101]

Hmmm ok, so this passes without reverting16348 (#16536).

Damn, how come? The failures that happen in the other branch don't even touch this code 🤔

[majocha]

Hmmm ok, so this passes without reverting16348 (#16536).

Damn, how come? The failures that happen in the other branch don't even touch this code 🤔

tbh, I now think this code just hides the problem better.

[majocha]

At this moment this is just trying things in the dark. I don't think it fixes anything. Actually I've seen the random test failure locally (TaskCancelledException), when running all tests in parallel in VS task explorer.