From 6c08230de8332edf6177081feab2e2ada94a682f Mon Sep 17 00:00:00 2001 From: Tom de Goede Date: Thu, 11 Nov 2021 12:27:23 +0100 Subject: [PATCH 1/4] Allow configProperties "STARTUP_HOOKS" and env "DOTNET_STARTUP_HOOKS" to both be present --- .../corehost/hostpolicy/hostpolicy_context.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/native/corehost/hostpolicy/hostpolicy_context.cpp b/src/native/corehost/hostpolicy/hostpolicy_context.cpp index 324c70e63ff272..11dcbcff5e3934 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_context.cpp +++ b/src/native/corehost/hostpolicy/hostpolicy_context.cpp @@ -281,11 +281,20 @@ int hostpolicy_context_t::initialize(hostpolicy_init_t &hostpolicy_init, const a pal::string_t startup_hooks; if (pal::getenv(_X("DOTNET_STARTUP_HOOKS"), &startup_hooks)) { - if (!coreclr_properties.add(common_property::StartUpHooks, startup_hooks.c_str())) + const pal::char_t *config_startup_hooks; + if (coreclr_properties.try_get(common_property::StartUpHooks, &config_startup_hooks)) { - log_duplicate_property_error(coreclr_property_bag_t::common_property_to_string(common_property::StartUpHooks)); - return StatusCode::LibHostDuplicateProperty; +#ifdef TARGET_UNIX + char separator = ':'; +#else + char separator = ';'; +#endif // TARGET_UNIX + + startup_hooks.push_back(separator); + startup_hooks.append(config_startup_hooks); } + + coreclr_properties.add(common_property::StartUpHooks, startup_hooks.c_str()); } // Single-File Bundle Probe From 5a613c0f2708c169c527b7949d802017ada922a9 Mon Sep 17 00:00:00 2001 From: Tom de Goede Date: Thu, 11 Nov 2021 12:28:14 +0100 Subject: [PATCH 2/4] Fix duplicate_property_error property names to reflect the actual duplicate --- src/native/corehost/hostpolicy/hostpolicy_context.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/native/corehost/hostpolicy/hostpolicy_context.cpp b/src/native/corehost/hostpolicy/hostpolicy_context.cpp index 11dcbcff5e3934..8c0b0b82f05ce4 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_context.cpp +++ b/src/native/corehost/hostpolicy/hostpolicy_context.cpp @@ -306,7 +306,7 @@ int hostpolicy_context_t::initialize(hostpolicy_init_t &hostpolicy_init, const a if (!coreclr_properties.add(common_property::BundleProbe, ptr_stream.str().c_str())) { - log_duplicate_property_error(coreclr_property_bag_t::common_property_to_string(common_property::StartUpHooks)); + log_duplicate_property_error(coreclr_property_bag_t::common_property_to_string(common_property::BundleProbe)); return StatusCode::LibHostDuplicateProperty; } } @@ -321,7 +321,7 @@ int hostpolicy_context_t::initialize(hostpolicy_init_t &hostpolicy_init, const a if (!coreclr_properties.add(common_property::PInvokeOverride, ptr_stream.str().c_str())) { - log_duplicate_property_error(coreclr_property_bag_t::common_property_to_string(common_property::StartUpHooks)); + log_duplicate_property_error(coreclr_property_bag_t::common_property_to_string(common_property::PInvokeOverride)); return StatusCode::LibHostDuplicateProperty; } } @@ -330,7 +330,7 @@ int hostpolicy_context_t::initialize(hostpolicy_init_t &hostpolicy_init, const a #if defined(HOSTPOLICY_EMBEDDED) if (!coreclr_properties.add(common_property::HostPolicyEmbedded, _X("true"))) { - log_duplicate_property_error(coreclr_property_bag_t::common_property_to_string(common_property::StartUpHooks)); + log_duplicate_property_error(coreclr_property_bag_t::common_property_to_string(common_property::HostPolicyEmbedded)); return StatusCode::LibHostDuplicateProperty; } #endif From 8fa136946e4a28898489b1270944e29338856b1c Mon Sep 17 00:00:00 2001 From: Tom de Goede Date: Mon, 15 Nov 2021 09:59:51 +0100 Subject: [PATCH 3/4] fixup! Allow configProperties "STARTUP_HOOKS" and env "DOTNET_STARTUP_HOOKS" to both be present --- .../HostActivation.Tests/StartupHooks.cs | 54 +++++++++++++++++++ .../hostpolicy/hostpolicy_context.cpp | 8 +-- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/StartupHooks.cs b/src/installer/tests/HostActivation.Tests/StartupHooks.cs index a0533a60fd4772..de20165071b6ff 100644 --- a/src/installer/tests/HostActivation.Tests/StartupHooks.cs +++ b/src/installer/tests/HostActivation.Tests/StartupHooks.cs @@ -13,6 +13,7 @@ public class StartupHooks : IClassFixture { private SharedTestState sharedTestState; private string startupHookVarName = "DOTNET_STARTUP_HOOKS"; + private string startupHookRuntimeConfigName = "STARTUP_HOOKS"; private string startupHookSupport = "System.StartupHookProvider.IsSupported"; public StartupHooks(StartupHooks.SharedTestState fixture) @@ -105,6 +106,59 @@ public void Muxer_activation_of_Multiple_StartupHooks_Succeeds() .And.HaveStdOutContaining("Hello World"); } + [Fact] + public void Muxer_activation_of_RuntimeConfig_StartupHook_Succeeds() + { + var fixture = sharedTestState.PortableAppFixture.Copy(); + var dotnet = fixture.BuiltDotnet; + var appDll = fixture.TestProject.AppDll; + + var startupHookFixture = sharedTestState.StartupHookFixture.Copy(); + var startupHookDll = startupHookFixture.TestProject.AppDll; + + RuntimeConfig.FromFile(fixture.TestProject.RuntimeConfigJson) + .WithProperty(startupHookRuntimeConfigName, startupHookDll) + .Save(); + + // RuntimeConfig defined startup hook + dotnet.Exec(appDll) + .CaptureStdOut() + .CaptureStdErr() + .Execute() + .Should().Pass() + .And.HaveStdOutContaining("Hello from startup hook!") + .And.HaveStdOutContaining("Hello World"); + } + + [Fact] + public void Muxer_activation_of_RuntimeConfig_And_Environment_StartupHooks_Succeeds() + { + var fixture = sharedTestState.PortableAppFixture.Copy(); + var dotnet = fixture.BuiltDotnet; + var appDll = fixture.TestProject.AppDll; + + var startupHookFixture = sharedTestState.StartupHookFixture.Copy(); + var startupHookDll = startupHookFixture.TestProject.AppDll; + + RuntimeConfig.FromFile(fixture.TestProject.RuntimeConfigJson) + .WithProperty(startupHookRuntimeConfigName, startupHookDll) + .Save(); + + var startupHook2Fixture = sharedTestState.StartupHookWithDependencyFixture.Copy(); + var startupHook2Dll = startupHook2Fixture.TestProject.AppDll; + + // RuntimeConfig and Environment startup hook + dotnet.Exec(appDll) + .EnvironmentVariable(startupHookVarName, startupHook2Dll) + .CaptureStdOut() + .CaptureStdErr() + .Execute() + .Should().Pass() + .And.HaveStdOutContaining("Hello from startup hook!") + .And.HaveStdOutContaining("Hello from startup hook with dependency!") + .And.HaveStdOutContaining("Hello World"); + } + // Empty startup hook variable [Fact] public void Muxer_activation_of_Empty_StartupHook_Variable_Succeeds() diff --git a/src/native/corehost/hostpolicy/hostpolicy_context.cpp b/src/native/corehost/hostpolicy/hostpolicy_context.cpp index 8c0b0b82f05ce4..1e85843407e07e 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_context.cpp +++ b/src/native/corehost/hostpolicy/hostpolicy_context.cpp @@ -284,13 +284,7 @@ int hostpolicy_context_t::initialize(hostpolicy_init_t &hostpolicy_init, const a const pal::char_t *config_startup_hooks; if (coreclr_properties.try_get(common_property::StartUpHooks, &config_startup_hooks)) { -#ifdef TARGET_UNIX - char separator = ':'; -#else - char separator = ';'; -#endif // TARGET_UNIX - - startup_hooks.push_back(separator); + startup_hooks.push_back(PATH_SEPARATOR); startup_hooks.append(config_startup_hooks); } From faba4365dec67913d4cf291800447bd5b8ae6651 Mon Sep 17 00:00:00 2001 From: Tom de Goede Date: Thu, 18 Nov 2021 15:39:44 +0100 Subject: [PATCH 4/4] Enforce env startup hook precedence over runtimeconfig startup hooks --- .../tests/HostActivation.Tests/StartupHooks.cs | 15 ++++++++++----- .../corehost/hostpolicy/hostpolicy_context.cpp | 2 ++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/StartupHooks.cs b/src/installer/tests/HostActivation.Tests/StartupHooks.cs index de20165071b6ff..3e1685356167f9 100644 --- a/src/installer/tests/HostActivation.Tests/StartupHooks.cs +++ b/src/installer/tests/HostActivation.Tests/StartupHooks.cs @@ -131,7 +131,7 @@ public void Muxer_activation_of_RuntimeConfig_StartupHook_Succeeds() } [Fact] - public void Muxer_activation_of_RuntimeConfig_And_Environment_StartupHooks_Succeeds() + public void Muxer_activation_of_RuntimeConfig_And_Environment_StartupHooks_SucceedsInExpectedOrder() { var fixture = sharedTestState.PortableAppFixture.Copy(); var dotnet = fixture.BuiltDotnet; @@ -147,16 +147,21 @@ public void Muxer_activation_of_RuntimeConfig_And_Environment_StartupHooks_Succe var startupHook2Fixture = sharedTestState.StartupHookWithDependencyFixture.Copy(); var startupHook2Dll = startupHook2Fixture.TestProject.AppDll; - // RuntimeConfig and Environment startup hook + // include any char to counter output from other threads such as in #57243 + const string wildcardPattern = @"[\r\n\s.]*"; + + // RuntimeConfig and Environment startup hooks in expected order dotnet.Exec(appDll) .EnvironmentVariable(startupHookVarName, startupHook2Dll) .CaptureStdOut() .CaptureStdErr() .Execute() .Should().Pass() - .And.HaveStdOutContaining("Hello from startup hook!") - .And.HaveStdOutContaining("Hello from startup hook with dependency!") - .And.HaveStdOutContaining("Hello World"); + .And.HaveStdOutMatching("Hello from startup hook with dependency!" + + wildcardPattern + + "Hello from startup hook!" + + wildcardPattern + + "Hello World"); } // Empty startup hook variable diff --git a/src/native/corehost/hostpolicy/hostpolicy_context.cpp b/src/native/corehost/hostpolicy/hostpolicy_context.cpp index 1e85843407e07e..22d26f25d92aff 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_context.cpp +++ b/src/native/corehost/hostpolicy/hostpolicy_context.cpp @@ -284,6 +284,8 @@ int hostpolicy_context_t::initialize(hostpolicy_init_t &hostpolicy_init, const a const pal::char_t *config_startup_hooks; if (coreclr_properties.try_get(common_property::StartUpHooks, &config_startup_hooks)) { + // env startup hooks shoold have precedence over config startup hooks + // therefore append config_startup_hooks AFTER startup_hooks startup_hooks.push_back(PATH_SEPARATOR); startup_hooks.append(config_startup_hooks); }