diff --git a/src/ef/ReflectionOperationExecutor.cs b/src/ef/ReflectionOperationExecutor.cs index 9314a4326dd..2155b237ab7 100644 --- a/src/ef/ReflectionOperationExecutor.cs +++ b/src/ef/ReflectionOperationExecutor.cs @@ -16,11 +16,11 @@ namespace Microsoft.EntityFrameworkCore.Tools; internal class ReflectionOperationExecutor : OperationExecutorBase { - private readonly object _executor; - private readonly Assembly _commandsAssembly; + private object _executor; + private Assembly _commandsAssembly; private const string ReportHandlerTypeName = "Microsoft.EntityFrameworkCore.Design.OperationReportHandler"; private const string ResultHandlerTypeName = "Microsoft.EntityFrameworkCore.Design.OperationResultHandler"; - private readonly Type _resultHandlerType; + private Type _resultHandlerType; private string? _efcoreVersion; #if NET private AssemblyLoadContext? _assemblyLoadContext; @@ -56,7 +56,9 @@ public ReflectionOperationExecutor( AppDomain.CurrentDomain.SetData("DataDirectory", dataDirectory); } +#if !NET AppDomain.CurrentDomain.AssemblyResolve += ResolveAssembly; +#endif #if NET _commandsAssembly = DesignAssemblyPath != null @@ -105,14 +107,20 @@ protected AssemblyLoadContext AssemblyLoadContext return _assemblyLoadContext; } - AssemblyLoadContext.Default.Resolving += (context, name) => + // Load the target and startup assemblies into a collectible context so they can be + // unloaded when this executor is disposed. The tool loads these assemblies to discover + // the DbContext, then shells out to 'dotnet publish', which rebuilds and overwrites the + // same bin\...\*.dll files. On Windows a loaded assembly file is locked, so the copy + // fails unless the file has been released first. See https://github.com/dotnet/efcore/issues/25555. + var assemblyLoadContext = new AssemblyLoadContext("EntityFrameworkCore.Tools", isCollectible: true); + assemblyLoadContext.Resolving += (context, name) => { var assemblyPath = Path.Combine(AppBasePath, name.Name + ".dll"); return File.Exists(assemblyPath) ? context.LoadFromAssemblyPath(assemblyPath) : null; }; - _assemblyLoadContext = AssemblyLoadContext.Default; + _assemblyLoadContext = assemblyLoadContext; - return AssemblyLoadContext.Default; + return assemblyLoadContext; } } #endif @@ -152,6 +160,7 @@ protected override void Execute(string operationName, object resultHandler, IDic resultHandler, arguments); +#if !NET private Assembly? ResolveAssembly(object? sender, ResolveEventArgs args) { var assemblyName = new AssemblyName(args.Name); @@ -173,7 +182,36 @@ protected override void Execute(string operationName, object resultHandler, IDic return null; } +#endif public override void Dispose() - => AppDomain.CurrentDomain.AssemblyResolve -= ResolveAssembly; + { +#if NET + // Drop every reference into the collectible context and unload it so the target and startup + // assemblies are released before 'dotnet publish' tries to overwrite their bin\...\*.dll + // files. See https://github.com/dotnet/efcore/issues/25555. + _executor = null!; + _resultHandlerType = null!; + _commandsAssembly = null!; + + if (_assemblyLoadContext is { IsCollectible: true } assemblyLoadContext) + { + _assemblyLoadContext = null; + + // The unload only completes once the GC observes that nothing references the context; + // force it here so the file lock is released by the time the caller starts publishing. + var weakReference = new WeakReference(assemblyLoadContext); + assemblyLoadContext.Unload(); + assemblyLoadContext = null; + + for (var i = 0; weakReference.IsAlive && i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + } +#else + AppDomain.CurrentDomain.AssemblyResolve -= ResolveAssembly; +#endif + } } diff --git a/test/ef.Tests/ReflectionOperationExecutorTest.cs b/test/ef.Tests/ReflectionOperationExecutorTest.cs new file mode 100644 index 00000000000..71335231c14 --- /dev/null +++ b/test/ef.Tests/ReflectionOperationExecutorTest.cs @@ -0,0 +1,99 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +extern alias eftool; +using System.Runtime.CompilerServices; +using System.Runtime.Loader; + +namespace Microsoft.EntityFrameworkCore.Tools; + +public class ReflectionOperationExecutorTest +{ + // Regression test for https://github.com/dotnet/efcore/issues/25555. + // The bundle command loads the user's assemblies into the tool process to discover the + // DbContext, then shells out `dotnet publish` which rebuilds and overwrites those same + // bin\...\*.dll files. On Windows a loaded assembly file is locked, so the copy fails. + // The executor must therefore release the user assemblies (unload its load context) when + // disposed, before publish runs. We can't observe the Windows lock cross-platform, but we + // can observe the portable substrate: after disposal the target assembly is no longer + // loaded in the process. + [Fact] + public void Dispose_unloads_the_target_assembly() + { + var targetDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(targetDir); + try + { + var build = new BuildSource { TargetDir = targetDir, Sources = { ["Nothing.cs"] = "public class Nothing { }" } }; + var targetPath = build.Build().TargetPath; + var designPath = Assembly.Load(new AssemblyName(OperationExecutorBase.DesignAssemblyName)).Location; + + RunOperation(targetPath, designPath); + + for (var i = 0; i < 10 && IsLoaded(targetPath); i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + + Assert.False(IsLoaded(targetPath), "Target assembly was still loaded after the executor was disposed."); + } + finally + { + DeleteDirectory(targetDir); + } + } + + private static void DeleteDirectory(string path) + { + // On Windows the just-unloaded assembly's file handle can be released slightly after the + // load context is collected, so a recursive delete may briefly fail with a sharing/access + // violation. This is a test-cleanup concern only (in the real tool, publish runs much + // later), so retry for a moment and then give up quietly rather than fail the test. + for (var i = 0; i < 20; i++) + { + try + { + Directory.Delete(path, recursive: true); + return; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + Thread.Sleep(50); + } + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void RunOperation(string targetPath, string designPath) + { + using var executor = new ReflectionOperationExecutor( + assembly: targetPath, + startupAssembly: targetPath, + designAssembly: designPath, + project: null, + projectDir: null, + dataDirectory: null, + rootNamespace: null, + language: null, + nullable: false, + remainingArguments: [], + reportHandler: new eftool::Microsoft.EntityFrameworkCore.Design.OperationReportHandler()); + + // Mirrors what the bundle command does: an operation that loads the target assembly and + // marshals its result back across the load-context boundary (the dynamic result handler). + _ = executor.GetContextTypes().ToList(); + + Assert.True(IsLoaded(targetPath), "Target assembly was not loaded by the operation; test would be vacuous."); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool IsLoaded(string targetPath) + => AssemblyLoadContext.All + .SelectMany(c => c.Assemblies) + .Any(a => !a.IsDynamic + && !string.IsNullOrEmpty(a.Location) + && string.Equals(a.Location, targetPath, StringComparison.OrdinalIgnoreCase)); +} diff --git a/test/ef.Tests/ef.Tests.csproj b/test/ef.Tests/ef.Tests.csproj index c8df176e8ed..a1b4ccbf355 100644 --- a/test/ef.Tests/ef.Tests.csproj +++ b/test/ef.Tests/ef.Tests.csproj @@ -4,6 +4,7 @@ $(DefaultNetCoreTargetFramework) Microsoft.EntityFrameworkCore.Tools true + true @@ -35,7 +36,7 @@ - +