Skip to content

FSI multi-emit unstable #18441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
majocha opened this issue Apr 3, 2025 · 3 comments
Open

FSI multi-emit unstable #18441

majocha opened this issue Apr 3, 2025 · 3 comments
Labels
Area-FSI Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Milestone

Comments

@majocha
Copy link
Contributor

majocha commented Apr 3, 2025

Some recently merged change made FSI multi-emit tests flaky:
https://dev.azure.com/dnceng-public/public/_build/results?buildId=1003058&view=ms.vss-test-web.build-test-results-tab&runId=26764208&resultId=104454&paneView=debug

[xUnit.net 00:11:53.04] Scripting\Interactive.fs(61,0): at Scripting.Interactive tests.Evaluation of multiple sessions should succeed(Boolean useMultiEmit)
Failed Scripting.Interactive tests.Evaluation of multiple sessions should succeed(useMultiEmit: True) [8 s]
Error Message:
System.TypeLoadException : Could not load type 'Test2' from assembly 'FSI-ASSEMBLY, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
Stack Trace:
at FSharp.Test.ScriptHelpers.TestHelpers.getValue(FSharpResult`2 value, FSharpDiagnostic[] errors) in D:\a_work\1\s\tests\FSharp.Test.Utilities\ScriptHelpers.fs:line 101
at Scripting.Interactive tests.Evaluation of multiple sessions should succeed(Boolean useMultiEmit) in D:\a_work\1\s\tests\FSharp.Compiler.ComponentTests\Scripting\Interactive.fs:line 61

Repro steps

Run Scripting tests in VS Test Explorer in parallel. Tests with useMultiEmit: true will fail.

This apparently happens when running multiple evaluation sessions concurrently when one of the sessions is single-emit, so impact is low.

@majocha
Copy link
Contributor Author

majocha commented Apr 3, 2025

Turns out this is not new.
The failure happens when we create two fsi sessions with and without multiEmit in the same context. Which is basically what the test run does.

To fix probably we could uniquely name the generated assemblies for each session, e.g. FSI-ASSEMBLY, FSI-ASSEMBLY1, FSI-ASSEMBLY2 and so on.

@majocha
Copy link
Contributor Author

majocha commented Apr 3, 2025

related #16118

The issue is that in tests we also have fsi sessions running concurrently but some of them are single-emit.

All of them install a AssemblyResolve handler in the AppDomain.
Now, when a multi-emit session asks to resolve a versioned FSI-ASSEMBLY but the single-emit handler is invoked first, the unversioned dynamic assembly is wrongly resolved here:

match fsiDynamicCompiler.FindDynamicAssembly(simpleAssemName, false) with
| Some asm when not (tcConfigB.fsiMultiAssemblyEmit) -> asm

and we get a TypeLoadException.

@T-Gro
Copy link
Member

T-Gro commented Apr 4, 2025

We know of people embedding fsi scripting in their respective apps.
Making it a supported scenario to have multiple scripting instances (for one loaded instance of FSharp.Compiler.Service) - assuming those instances can differ in their multiEmit setting - would be a good change.

@T-Gro T-Gro added Area-FSI Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. and removed Needs-Triage labels Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-FSI Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
Status: New
Development

Successfully merging a pull request may close this issue.

2 participants