From 6607db54ae8c3978517ad3a1900082faa5e139aa Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Thu, 1 Aug 2024 22:43:43 -0400 Subject: [PATCH 1/7] fix: Don't fail metadata updates on missing assemblies --- .../DotNetDeltaApplier/HotReloadAgent.cs | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/src/BuiltInTools/DotNetDeltaApplier/HotReloadAgent.cs b/src/BuiltInTools/DotNetDeltaApplier/HotReloadAgent.cs index 62182db3b0bd..f11b430386d9 100644 --- a/src/BuiltInTools/DotNetDeltaApplier/HotReloadAgent.cs +++ b/src/BuiltInTools/DotNetDeltaApplier/HotReloadAgent.cs @@ -82,24 +82,36 @@ private UpdateHandlerActions GetMetadataUpdateHandlerActions() var handlerActions = new UpdateHandlerActions(); foreach (var assembly in sortedAssemblies) { - foreach (var attr in assembly.GetCustomAttributesData()) + try { - // Look up the attribute by name rather than by type. This would allow netstandard targeting libraries to - // define their own copy without having to cross-compile. - if (attr.AttributeType.FullName != "System.Reflection.Metadata.MetadataUpdateHandlerAttribute") + foreach (var attr in assembly.GetCustomAttributesData()) { - continue; + // Look up the attribute by name rather than by type. This would allow netstandard targeting libraries to + // define their own copy without having to cross-compile. + if (attr.AttributeType.FullName != "System.Reflection.Metadata.MetadataUpdateHandlerAttribute") + { + continue; + } + + IList ctorArgs = attr.ConstructorArguments; + if (ctorArgs.Count != 1 || + ctorArgs[0].Value is not Type handlerType) + { + _log($"'{attr}' found with invalid arguments."); + continue; + } + + GetHandlerActions(handlerActions, handlerType); } - - IList ctorArgs = attr.ConstructorArguments; - if (ctorArgs.Count != 1 || - ctorArgs[0].Value is not Type handlerType) - { - _log($"'{attr}' found with invalid arguments."); - continue; - } - - GetHandlerActions(handlerActions, handlerType); + } + catch (Exception e) + { + // In cross-platform scenarios, such as debugging in VS through WSL, Roslyn + // runs on Windows, and the agent runs on Linux. Assemblies accessible to Windows + // may not be available or loaded on linux (such as WPF's assemblies). + // In such case, we can ignore the assemblies and continue enumerating handlers for + // the rest of the assemblies of current domain. + _log($"'{assembly.FullName}' is not loaded ({e.Message})"); } } From 9e53fec58dfba9936d6aa994d06c46e376d2ec48 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Thu, 1 Aug 2024 22:45:10 -0400 Subject: [PATCH 2/7] tests: Add tests for assembly load failure --- .../App/App.csproj | 13 +++++++ .../App/Program.cs | 33 ++++++++++++++++ .../App/Update.cs | 13 +++++++ .../WatchAppMissingAssemblyFailure/Dep/Dep.cs | 7 ++++ .../Dep/Dep.csproj | 8 ++++ .../HotReload/ApplyDeltaTests.cs | 39 ++++++++++++++++++- 6 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/App.csproj create mode 100644 test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Program.cs create mode 100644 test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Update.cs create mode 100644 test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs create mode 100644 test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.csproj diff --git a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/App.csproj b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/App.csproj new file mode 100644 index 000000000000..4505e48191a6 --- /dev/null +++ b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/App.csproj @@ -0,0 +1,13 @@ + + + + Exe + $(CurrentTargetFramework) + enable + + + + + + + diff --git a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Program.cs b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Program.cs new file mode 100644 index 000000000000..be686c646820 --- /dev/null +++ b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Program.cs @@ -0,0 +1,33 @@ +using System.Diagnostics; +using System.Reflection.Metadata; + +[assembly: MetadataUpdateHandler(typeof(UpdateHandler))] + +// delete the dependency dll to cause load failure of DepSubType +var depPath = Path.Combine(Path.GetDirectoryName(typeof(Program).Assembly.Location!)!, "Dep.dll"); +File.Delete(depPath); +Console.WriteLine($"File deleted: {depPath}"); + +while (true) +{ + lock (UpdateHandler.Guard) + { + Printer.Print(); + } + + Thread.Sleep(100); +} + +static class UpdateHandler +{ + // Lock to avoid the updated Print method executing concurrently with the update handler. + public static object Guard = new object(); + + public static void UpdateApplication(Type[] types) + { + lock (Guard) + { + Console.WriteLine($"Updated types: {(types == null ? "" : types.Length == 0 ? "" : string.Join(",", types.Select(t => t.Name)))}"); + } + } +} diff --git a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Update.cs b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Update.cs new file mode 100644 index 000000000000..5df8d231b148 --- /dev/null +++ b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Update.cs @@ -0,0 +1,13 @@ +using System; + +class DepSubType : Dep +{ + int F() => 1; +} + +class Printer +{ + public static void Print() + => Console.WriteLine("Hello!"); +} + diff --git a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs new file mode 100644 index 000000000000..b2db74fcbb3c --- /dev/null +++ b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs @@ -0,0 +1,7 @@ +public class Dep +{ + void F() + { + Console.WriteLine(1); + } +} diff --git a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.csproj b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.csproj new file mode 100644 index 000000000000..78589723776a --- /dev/null +++ b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.csproj @@ -0,0 +1,8 @@ + + + + $(CurrentTargetFramework) + enable + + + diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index 53089a584dba..302c8aa13ded 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -78,7 +78,7 @@ public async Task HandleTypeLoadFailure() await App.AssertWaitingForChanges(); - var newSrc = """ + var newSrc = /* lang=c#-test */""" class DepSubType : Dep { int F() => 2; @@ -120,5 +120,42 @@ public async Task BlazorWasm() //UpdateSourceFile(Path.Combine(testAsset.Path, "Pages", "Index.razor"), newSource); //await App.AssertOutputLineStartsWith(MessageDescriptor.HotReloadSucceeded); } + + // Test is timing out on .NET Framework: https://github.com/dotnet/sdk/issues/41669 + [CoreMSBuildOnlyFact] + public async Task HandleMissingAssemblyFailure() + { + var testAsset = TestAssets.CopyTestAsset("WatchAppMissingAssemblyFailure") + .WithSource(); + + await App.StartWatcherAsync(testAsset, "App"); + + await App.WaitForSessionStarted(); + + var newSrc = /* lang=c#-test */""" + class DepSubType : Dep + { + int F() => 2; + } + + class Printer + { + public static void Print() + { + Console.WriteLine("Changed!"); + } + } + """; + + // Delete all files in testAsset.Path named Dep.dll + foreach (var depDll in Directory.GetFiles(testAsset.Path, "Dep.dll", SearchOption.AllDirectories)) + { + File.Delete(depDll); + } + + File.WriteAllText(Path.Combine(testAsset.Path, "App", "Update.cs"), newSrc); + + await App.AssertOutputLineStartsWith("Updated types: Printer"); + } } } From 1bcdc05906286789993c2682b504aaf342ccbea2 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Fri, 2 Aug 2024 10:57:50 -0400 Subject: [PATCH 3/7] test: Add handler validation --- .../App/Program.cs | 1 + .../App/Update.cs | 2 +- .../WatchAppMissingAssemblyFailure/Dep/Dep.cs | 18 +++++++++++++++++- .../HotReload/ApplyDeltaTests.cs | 14 ++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Program.cs b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Program.cs index be686c646820..627d78708d6e 100644 --- a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Program.cs +++ b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Program.cs @@ -2,6 +2,7 @@ using System.Reflection.Metadata; [assembly: MetadataUpdateHandler(typeof(UpdateHandler))] +[assembly: MetadataUpdateHandler(typeof(Dep.UpdateHandler))] // delete the dependency dll to cause load failure of DepSubType var depPath = Path.Combine(Path.GetDirectoryName(typeof(Program).Assembly.Location!)!, "Dep.dll"); diff --git a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Update.cs b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Update.cs index 5df8d231b148..a589b9f27b7a 100644 --- a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Update.cs +++ b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/App/Update.cs @@ -1,6 +1,6 @@ using System; -class DepSubType : Dep +class DepSubType : Dep.DepType { int F() => 1; } diff --git a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs index b2db74fcbb3c..7419d12485af 100644 --- a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs +++ b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs @@ -1,7 +1,23 @@ -public class Dep +namespace Dep; + +public class DepType { void F() { Console.WriteLine(1); } } + +static class UpdateHandler +{ + // Lock to avoid the updated Print method executing concurrently with the update handler. + public static object Guard = new object(); + + public static void UpdateApplication(Type[] types) + { + lock (Guard) + { + Console.WriteLine($"Dep Updated types: {(types == null ? "" : types.Length == 0 ? "" : string.Join(",", types.Select(t => t.Name)))}"); + } + } +} diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index 302c8aa13ded..9145acb3db74 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -145,6 +145,20 @@ public static void Print() Console.WriteLine("Changed!"); } } + + static class UpdateHandler + { + // Lock to avoid the updated Print method executing concurrently with the update handler. + public static object Guard = new object(); + + public static void UpdateApplication(Type[] types) + { + lock (Guard) + { + Console.WriteLine($"Dep Updated types: {(types == null ? "" : types.Length == 0 ? "" : string.Join(",", types.Select(t => t.Name)))}"); + } + } + } """; // Delete all files in testAsset.Path named Dep.dll From f825c3331cdb8e62cb359265290c437c44aa49be Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Fri, 2 Aug 2024 11:47:12 -0400 Subject: [PATCH 4/7] choe: Make update handler public --- .../TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs | 2 +- test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs index 7419d12485af..527201d3ef32 100644 --- a/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs +++ b/test/TestAssets/TestProjects/WatchAppMissingAssemblyFailure/Dep/Dep.cs @@ -8,7 +8,7 @@ void F() } } -static class UpdateHandler +public static class UpdateHandler { // Lock to avoid the updated Print method executing concurrently with the update handler. public static object Guard = new object(); diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index 9145acb3db74..35f4e14f0c5a 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -146,7 +146,7 @@ public static void Print() } } - static class UpdateHandler + public static class UpdateHandler { // Lock to avoid the updated Print method executing concurrently with the update handler. public static object Guard = new object(); From 7b28d6deb934720eda61a9cbe6fd1a02f3c0d919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Laban?= Date: Fri, 2 Aug 2024 15:18:40 -0400 Subject: [PATCH 5/7] chore: adjust update content --- test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index 35f4e14f0c5a..0e6342d373aa 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -133,7 +133,7 @@ public async Task HandleMissingAssemblyFailure() await App.WaitForSessionStarted(); var newSrc = /* lang=c#-test */""" - class DepSubType : Dep + class DepSubType : Dep.DepType { int F() => 2; } From 632e844ffe3f6e9b8ac2f725fc719b0a8708a9ff Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Tue, 27 Aug 2024 13:45:50 -0400 Subject: [PATCH 6/7] chore: Adjust for newer APIs --- test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index 0e6342d373aa..1ad96d0c3ce3 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -78,7 +78,7 @@ public async Task HandleTypeLoadFailure() await App.AssertWaitingForChanges(); - var newSrc = /* lang=c#-test */""" + var newSrc = """ class DepSubType : Dep { int F() => 2; @@ -128,9 +128,9 @@ public async Task HandleMissingAssemblyFailure() var testAsset = TestAssets.CopyTestAsset("WatchAppMissingAssemblyFailure") .WithSource(); - await App.StartWatcherAsync(testAsset, "App"); + App.Start(testAsset, [], "App"); - await App.WaitForSessionStarted(); + await App.AssertWaitingForChanges(); var newSrc = /* lang=c#-test */""" class DepSubType : Dep.DepType From a336fe74895563a26645744cafed2336bda1a71e Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Tue, 10 Sep 2024 09:08:44 -0400 Subject: [PATCH 7/7] chore: Adjust for disabled test https://github.com/dotnet/sdk/issues/42850 --- test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index 1ad96d0c3ce3..ad6eb1b0e286 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -122,7 +122,7 @@ public async Task BlazorWasm() } // Test is timing out on .NET Framework: https://github.com/dotnet/sdk/issues/41669 - [CoreMSBuildOnlyFact] + [CoreMSBuildOnlyFact(Skip = "https://github.com/dotnet/sdk/issues/42850")] public async Task HandleMissingAssemblyFailure() { var testAsset = TestAssets.CopyTestAsset("WatchAppMissingAssemblyFailure")