fix: Dynamic code execution stays fast with system .NET 6 installed#988
Conversation
Disable .NET multilevel lookup for the shared Roslyn worker process so Unity's bundled runtime is used consistently with the references selected during worker compilation. Add coverage for the worker runtime environment setting to guard against regressions.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 36 minutes and 32 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changes add functionality to disable .NET multilevel runtime lookup in the shared Roslyn compiler worker host, preventing system-wide .NET installations from interfering with Unity's bundled runtime. A new test verifies this environment variable configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/src/Editor/Compilation/SharedRoslynCompilerWorkerHost.cs (1)
542-551:⚠️ Potential issue | 🟡 MinorAdd
ConfigureWorkerDotnetRuntimeEnvironmentto the worker-assembly build process at line 551.
CompileWorkerAssemblyspawns the Roslyn compiler viaProcessStartInfo(lines 542–551) without callingConfigureWorkerDotnetRuntimeEnvironment, whereasCreateWorkerStartInfo(lines 491–508) correctly applies this guard when spawning the compiled worker. The compiler DLL'sruntimeconfig.jsonis subject to the same multilevel-lookup probing that this PR addresses, and withoutDOTNET_MULTILEVEL_LOOKUP=0, the process can fail withFileLoadExceptionif a system .NET 6 runtime is discovered before the bundled .NET 8 runtime, blocking the worker fast path on the first compile after shutdown.🛠️ Proposed change
ProcessStartInfo startInfo = new ProcessStartInfo { FileName = externalCompilerPaths.DotnetHostPath, Arguments = $"{QuoteCommandLineArgument(externalCompilerPaths.CompilerDllPath)} @{QuoteCommandLineArgument(workerCompileResponseFilePath)}", WorkingDirectory = Path.GetDirectoryName(workerSourcePath), UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, CreateNoWindow = true }; + ConfigureWorkerDotnetRuntimeEnvironment(startInfo);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Packages/src/Editor/Compilation/SharedRoslynCompilerWorkerHost.cs` around lines 542 - 551, CompileWorkerAssembly creates a ProcessStartInfo (startInfo) to invoke the Roslyn compiler but does not apply the same DOTNET_MULTILEVEL_LOOKUP guard used in CreateWorkerStartInfo; to fix, call ConfigureWorkerDotnetRuntimeEnvironment(startInfo) after the ProcessStartInfo is constructed (the same place CreateWorkerStartInfo applies it) so the compiler process has the multilevel lookup disabled and won’t pick up a system .NET runtime before the bundled one.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Packages/src/Editor/Compilation/SharedRoslynCompilerWorkerHost.cs`:
- Around line 542-551: CompileWorkerAssembly creates a ProcessStartInfo
(startInfo) to invoke the Roslyn compiler but does not apply the same
DOTNET_MULTILEVEL_LOOKUP guard used in CreateWorkerStartInfo; to fix, call
ConfigureWorkerDotnetRuntimeEnvironment(startInfo) after the ProcessStartInfo is
constructed (the same place CreateWorkerStartInfo applies it) so the compiler
process has the multilevel lookup disabled and won’t pick up a system .NET
runtime before the bundled one.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c36206ad-7bc1-45ed-9c3d-1a64ff155c94
⛔ Files ignored due to path filters (1)
Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs.metais excluded by none and included by none
📒 Files selected for processing (2)
Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.csPackages/src/Editor/Compilation/SharedRoslynCompilerWorkerHost.cs
Use the same DOTNET_MULTILEVEL_LOOKUP guard when launching the Roslyn compiler that builds the shared worker, so the initial worker build cannot select a system runtime before Unity's bundled runtime.
|
Addressed the CodeRabbit review comment in The Roslyn compiler process used by Verification performed after the change:
|
Summary
Reproduction
mainwith Unity6000.4.3f1and systemMicrosoft.NETCore.App 6.0.36installed.execute-dynamic-code shared Roslyn worker failed to operate correctlyand falls back to one-shot compiler execution.Verification
uloop compile --force-recompile false --wait-for-domain-reload truecompleted with 0 errors and 0 warnings.SharedRoslynCompilerWorkerHostTests.ConfigureWorkerDotnetRuntimeEnvironment_WhenCalled_ShouldDisableMultilevelLookuppassed.6000.4.3f1and system.NET 6.0.36,execute-dynamic-codereturned6000.4.3f1successfully.uloop get-logs --search-text "shared Roslyn worker" --max-count 50 --include-stack-trace truereturnedTotalCount: 0.uloop get-logs --log-type Error --max-count 50 --include-stack-trace falsereturnedTotalCount: 0.Fixes #985