From 15c2cc44d48dc25b71dd94fa700152790f80b643 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Sat, 26 Jun 2021 03:30:00 -0400 Subject: [PATCH] Miscellaneous MetadataUpdater cleanups (#54751) --- .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 15 ++++++++----- .../System.Runtime.Loader/tests/ApplyUpdateUtil.cs | 25 +++++++++++++--------- .../System/Reflection/Metadata/MetadataUpdater.cs | 6 +++--- src/mono/mono/metadata/icall-decl.h | 2 +- src/mono/mono/metadata/icall.c | 6 ++++-- 5 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index e09eaa0..c2f06b8 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -14,10 +14,9 @@ namespace System.Reflection.Metadata /// testsuite runs each test in sequence, loading the corresponding /// assembly, applying an update to it and observing the results. [Collection(nameof(ApplyUpdateUtil.NoParallelTests))] - [ConditionalClass(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))] public class ApplyUpdateTest { - [Fact] + [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))] [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] void StaticMethodBodyUpdate() { @@ -40,7 +39,7 @@ namespace System.Reflection.Metadata }); } - [Fact] + [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))] [ActiveIssue("https://github.com/dotnet/runtime/issues/52993", TestRuntimes.Mono)] void ClassWithCustomAttributes() { @@ -122,12 +121,18 @@ namespace System.Reflection.Metadata Assert.Equal(typeof(string), result.GetType()); } - [Fact] - [ActiveIssue("Returns true on mono", TestRuntimes.Mono)] + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.TestUsingRemoteExecutor))] public static void IsSupported() { bool result = MetadataUpdater.IsSupported; Assert.False(result); } + + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.TestUsingLaunchEnvironment))] + public static void IsSupported2() + { + bool result = MetadataUpdater.IsSupported; + Assert.True(result); + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs index 235e94b..36d04c4 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs @@ -19,20 +19,27 @@ namespace System.Reflection.Metadata /// /// We need: /// 1. Either DOTNET_MODIFIABLE_ASSEMBLIES=debug is set, or we can use the RemoteExecutor to run a child process with that environment; and, - /// 2. Either Mono in a supported configuration (interpreter as the execution engine, and the hot reload feature enabled), or CoreCLR; and, + /// 2. Either Mono in a supported configuration (interpreter as the execution engine, and the hot reload component enabled), or CoreCLR; and, /// 3. The test assemblies are compiled with Debug information (this is configured by setting EmitDebugInformation in ApplyUpdate\Directory.Build.props) public static bool IsSupported => (IsModifiableAssembliesSet || IsRemoteExecutorSupported) && (!IsMonoRuntime || IsSupportedMonoConfiguration); + /// true if the current runtime was not launched with the apropriate settings for applying + /// updates (DOTNET_MODIFIABLE_ASSEMBLIES unset), but we can use the remote executor to + /// launch a child process that has the right setting. + public static bool TestUsingRemoteExecutor => IsRemoteExecutorSupported && !IsModifiableAssembliesSet; + + /// true if the current runtime was launched with the appropriate settings for applying + /// updates (DOTNET_MODIFIABLE_ASSEMBLIES set, and if Mono, the interpreter is enabled). + public static bool TestUsingLaunchEnvironment => (!IsMonoRuntime || IsSupportedMonoConfiguration) && IsModifiableAssembliesSet; + public static bool IsModifiableAssembliesSet => String.Equals(DotNetModifiableAssembliesValue, Environment.GetEnvironmentVariable(DotNetModifiableAssembliesSwitch), StringComparison.InvariantCultureIgnoreCase); // static cctor for RemoteExecutor throws on wasm. public static bool IsRemoteExecutorSupported => !RuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER")) && RemoteExecutor.IsSupported; - // copied from https://github.com/dotnet/arcade/blob/6cc4c1e9e23d5e65e88a8a57216b3d91e9b3d8db/src/Microsoft.DotNet.XUnitExtensions/src/DiscovererHelpers.cs#L16-L17 - private static readonly Lazy s_isMonoRuntime = new Lazy(() => Type.GetType("Mono.RuntimeStructs") != null); - public static bool IsMonoRuntime => s_isMonoRuntime.Value; + public static bool IsMonoRuntime => PlatformDetection.IsMonoRuntime; private static readonly Lazy s_isSupportedMonoConfiguration = new Lazy(CheckSupportedMonoConfiguration); @@ -41,7 +48,7 @@ namespace System.Reflection.Metadata // Not every build of Mono supports ApplyUpdate internal static bool CheckSupportedMonoConfiguration() { - // check that interpreter is enabled, and the build has hot reload capabilities enabled. + // check that interpreter is enabled, and the build has hot reload capabilities enabled. var isInterp = RuntimeFeature.IsDynamicCodeSupported && !RuntimeFeature.IsDynamicCodeCompiled; return isInterp && HasApplyUpdateCapabilities(); } @@ -75,7 +82,7 @@ namespace System.Reflection.Metadata string basename = assm.Location; if (basename == "") basename = assm.GetName().Name + ".dll"; - Console.Error.WriteLine($"Apply Delta Update for {basename}, revision {count}"); + Console.Error.WriteLine($"Applying metadata update for {basename}, revision {count}"); string dmeta_name = $"{basename}.{count}.dmeta"; string dil_name = $"{basename}.{count}.dil"; @@ -86,8 +93,6 @@ namespace System.Reflection.Metadata MetadataUpdater.ApplyUpdate(assm, dmeta_data, dil_data, dpdb_data); } - internal static bool UseRemoteExecutor => !IsModifiableAssembliesSet; - internal static void AddRemoteInvokeOptions (ref RemoteInvokeOptions options) { options = options ?? new RemoteInvokeOptions(); @@ -101,7 +106,7 @@ namespace System.Reflection.Metadata public static void TestCase(Action testBody, RemoteInvokeOptions options = null) { - if (UseRemoteExecutor) + if (TestUsingRemoteExecutor) { Console.Error.WriteLine ($"Running test using RemoteExecutor"); AddRemoteInvokeOptions(ref options); @@ -123,7 +128,7 @@ namespace System.Reflection.Metadata string arg1, RemoteInvokeOptions options = null) { - if (UseRemoteExecutor) + if (TestUsingRemoteExecutor) { AddRemoteInvokeOptions(ref options); RemoteExecutor.Invoke(testBody, arg1, options).Dispose(); diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs index ea7035ee..973eb43 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs @@ -49,17 +49,17 @@ namespace System.Reflection.Metadata internal static string GetCapabilities() => s_ApplyUpdateCapabilities.Value; - public static bool IsSupported { get; } = ApplyUpdateEnabled() != 0; + public static bool IsSupported { get; } = ApplyUpdateEnabled(justComponentCheck: 0) != 0; private static Lazy s_ApplyUpdateCapabilities = new Lazy(() => InitializeApplyUpdateCapabilities()); private static string InitializeApplyUpdateCapabilities() { - return ApplyUpdateEnabled() != 0 ? "Baseline" : string.Empty ; + return ApplyUpdateEnabled(justComponentCheck: 1) != 0 ? "Baseline" : string.Empty ; } [MethodImpl (MethodImplOptions.InternalCall)] - private static extern int ApplyUpdateEnabled (); + private static extern int ApplyUpdateEnabled (int justComponentCheck); [MethodImpl (MethodImplOptions.InternalCall)] private static unsafe extern void ApplyUpdate_internal (IntPtr base_assm, byte* dmeta_bytes, int dmeta_length, byte *dil_bytes, int dil_length, byte *dpdb_bytes, int dpdb_length); diff --git a/src/mono/mono/metadata/icall-decl.h b/src/mono/mono/metadata/icall-decl.h index b78d7c6..d6d052b 100644 --- a/src/mono/mono/metadata/icall-decl.h +++ b/src/mono/mono/metadata/icall-decl.h @@ -243,6 +243,6 @@ ICALL_EXPORT void ves_icall_System_Runtime_Intrinsics_X86_X86Base___cpuidex (int #endif ICALL_EXPORT void ves_icall_AssemblyExtensions_ApplyUpdate (MonoAssembly *assm, gconstpointer dmeta_bytes, int32_t dmeta_len, gconstpointer dil_bytes, int32_t dil_len, gconstpointer dpdb_bytes, int32_t dpdb_len); -ICALL_EXPORT gint32 ves_icall_AssemblyExtensions_ApplyUpdateEnabled (void); +ICALL_EXPORT gint32 ves_icall_AssemblyExtensions_ApplyUpdateEnabled (gint32 just_component_check); #endif // __MONO_METADATA_ICALL_DECL_H__ diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index c6e3905..51a8fc0 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -5787,9 +5787,11 @@ ves_icall_AssemblyExtensions_ApplyUpdate (MonoAssembly *assm, mono_error_set_pending_exception (error); } -gint32 ves_icall_AssemblyExtensions_ApplyUpdateEnabled (void) +gint32 ves_icall_AssemblyExtensions_ApplyUpdateEnabled (gint32 just_component_check) { - return mono_metadata_update_available () && mono_metadata_update_enabled (NULL); + // if just_component_check is true, we only care whether the hot_reload component is enabled, + // not whether the environment is appropriately setup to apply updates. + return mono_metadata_update_available () && (just_component_check || mono_metadata_update_enabled (NULL)); } MonoBoolean -- 2.7.4