Fix for changed host behaviour on app crash (#34224)
authorElinor Fung <47805090+elinor-fung@users.noreply.github.com>
Mon, 30 Mar 2020 16:47:26 +0000 (09:47 -0700)
committerGitHub <noreply@github.com>
Mon, 30 Mar 2020 16:47:26 +0000 (09:47 -0700)
* Switch back to only handling synchronous exceptions in host components

Add native hosting tests for components with unhandled exceptions.

* Switch setting /EHa to be for coreclr instead of shared default

12 files changed:
eng/native/configurecompiler.cmake
src/coreclr/CMakeLists.txt
src/coreclr/tests/CMakeLists.txt
src/installer/corehost/CMakeLists.txt
src/installer/corehost/cli/hostpolicy/breadcrumbs.cpp
src/installer/corehost/cli/hostpolicy/breadcrumbs.h
src/installer/corehost/cli/test/nativehost/host_context_test.cpp
src/installer/corehost/cli/test/nativehost/nativehost.cpp
src/installer/test/Assets/TestProjects/ComponentWithNoDependencies/Component.cs
src/installer/test/HostActivation.Tests/Constants.cs
src/installer/test/HostActivation.Tests/NativeHosting/ApplicationExecution.cs [new file with mode: 0644]
src/installer/test/HostActivation.Tests/NativeHosting/ComponentActivation.cs

index d93fe4b..a31b7b1 100644 (file)
@@ -413,7 +413,6 @@ if (MSVC)
 
   # The following options are set by the razzle build
   add_compile_options(/TP) # compile all files as C++
-  add_compile_options(/d2Zi+) # make optimized builds debugging easier
   add_compile_options(/nologo) # Suppress Startup Banner
   add_compile_options(/W3) # set warning level to 3
   add_compile_options(/WX) # treat warnings as errors
@@ -422,7 +421,6 @@ if (MSVC)
   add_compile_options(/U_MT) # undefine the predefined _MT macro
   add_compile_options(/GF) # enable read-only string pooling
   add_compile_options(/Gm-) # disable minimal rebuild
-  add_compile_options(/EHa) # enable C++ EH (w/ SEH exceptions)
   add_compile_options(/Zp8) # pack structs on 8-byte boundary
   add_compile_options(/Gy) # separate functions for linker
   add_compile_options(/Zc:wchar_t-) # C++ language conformance: wchar_t is NOT the native type, but a typedef
index d154553..efad610 100644 (file)
@@ -13,6 +13,7 @@ if (CLR_CMAKE_HOST_WIN32)
 endif (CLR_CMAKE_HOST_WIN32)
 
 if(MSVC)
+  add_compile_options(/EHa) # enable C++ EH (w/ SEH exceptions)
   set(CMAKE_CXX_STANDARD_LIBRARIES "") # do not link against standard win32 libs i.e. kernel32, uuid, user32, etc.
 endif (MSVC)
 
index 3b82b26..53dbb85 100644 (file)
@@ -27,7 +27,7 @@ if (CLR_CMAKE_HOST_WIN32)
     # 5027 - move assignment operator was implicitly defined as deleted
     # 5039 - pointer or reference to potentially throwing function passed to extern C function under -EHc. Undefined behavior may occur if this function throws an exception.
     add_compile_options(-wd4100 -wd4514 -wd4625 -wd4626 -wd4668 -wd4710 -wd4711 -wd4774 -wd4820 -wd5025 -wd5026 -wd5027 -wd5039)
-    set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc")
+    add_compile_options(/EHa) # enable C++ EH (w/ SEH exceptions)
 endif()
 
 MACRO(SUBDIRLIST result curdir)
index 63abbbe..43a59a1 100644 (file)
@@ -6,6 +6,9 @@ include(${CLR_ENG_NATIVE_DIR}/configurecompiler.cmake)
 
 if(MSVC)
     add_compile_options(/W1)
+
+    # Host components don't try to handle asynchronous exceptions
+    add_compile_options(/EHsc)
 endif()
 
 add_subdirectory(cli)
index 8338f26..da35ec8 100644 (file)
@@ -19,8 +19,7 @@ breadcrumb_writer_t::breadcrumb_writer_t(std::unordered_set<pal::string_t> &file
     }
 }
 
-// Begin breadcrumb writing: write synchronously or launch a
-// thread to write breadcrumbs.
+// Begin breadcrumb writing: launch a thread to write breadcrumbs.
 std::shared_ptr<breadcrumb_writer_t> breadcrumb_writer_t::begin_write(std::unordered_set<pal::string_t> &files)
 {
     trace::verbose(_X("--- Begin breadcrumb write"));
index 386c1af..10756c1 100644 (file)
@@ -11,7 +11,12 @@ class breadcrumb_writer_t
 {
 public:
     breadcrumb_writer_t(std::unordered_set<pal::string_t> &files);
+
+    // Starts writing breadcrumbs on a new thread if necessary.
+    // If end_write is not called on the returned instance before it is destructed, the process will be terminated.
     static std::shared_ptr<breadcrumb_writer_t> begin_write(std::unordered_set<pal::string_t> &files);
+
+    // Waits for the breadcrumb thread to finish writing.
     void end_write();
 
 private:
index 110fd84..708fa2b 100644 (file)
@@ -175,6 +175,56 @@ namespace
         return rc == StatusCode::Success && rcClose == StatusCode::Success;
     }
 
+    int run_app_with_try_except(
+        hostfxr_run_app_fn run_app,
+        const hostfxr_handle handle,
+        pal::stringstream_t &test_output)
+    {
+#if defined(WIN32)
+        __try
+#endif
+        {
+            int rc = run_app(handle);
+            if (rc != StatusCode::Success)
+                test_output << _X("hostfxr_run_app failed: ") << std::hex << std::showbase << rc << std::endl;
+
+            return rc;
+        }
+#if defined(WIN32)
+        __except(GetExceptionCode() != 0)
+        {
+            test_output << _X("hostfxr_run_app threw exception: ") << std::hex << std::showbase << GetExceptionCode() << std::endl;
+        }
+#endif
+
+        return -1;
+    }
+
+    int call_delegate_with_try_except(
+        component_entry_point_fn component_entry_point,
+        const pal::char_t *method_name,
+        const pal::char_t *log_prefix,
+        pal::stringstream_t &test_output)
+    {
+#if defined(WIN32)
+        __try
+#endif
+        {
+            int result = component_entry_point((void*)(static_cast<size_t>(0xdeadbeef)), 42);
+            test_output << log_prefix << method_name << _X(" delegate result: ") << std::hex << std::showbase << result << std::endl;
+
+            return StatusCode::Success;
+        }
+#if defined(WIN32)
+        __except(GetExceptionCode() != 0)
+        {
+            test_output << log_prefix << method_name << _X(" delegate threw exception: ") << std::hex << std::showbase << GetExceptionCode() << std::endl;
+        }
+#endif
+
+        return -1;
+    }
+
     bool load_assembly_and_get_function_pointer_test(
         const hostfxr_exports &hostfxr,
         const pal::char_t *config_path,
@@ -231,10 +281,7 @@ namespace
                 else
                 {
                     test_output << log_prefix << _X("load_assembly_and_get_function_pointer succeeded: ") << std::hex << std::showbase << rc << std::endl;
-
-                    int result = componentEntryPointDelegate((void*)(static_cast<size_t>(0xdeadbeef)), 42);
-
-                    test_output << log_prefix << method_name << _X(" delegate result: ") << std::hex << std::showbase << result << std::endl;
+                    rc = call_delegate_with_try_except(componentEntryPointDelegate, method_name, log_prefix, test_output);
                 }
             }
         }
@@ -296,13 +343,11 @@ bool host_context_test::app(
 
     inspect_modify_properties(check_properties, hostfxr, handle, argc, argv, app_log_prefix, test_output);
 
-    rc = hostfxr.run_app(handle);
-    if (rc != StatusCode::Success)
-        test_output << _X("hostfxr_run_app failed: ") << std::hex << std::showbase << rc << std::endl;
+    rc = run_app_with_try_except(hostfxr.run_app, handle, test_output);
 
     int rcClose = hostfxr.close(handle);
     if (rcClose != StatusCode::Success)
-        test_output << _X("hostfxr_close failed: ") << std::hex << std::showbase << rc  << std::endl;
+        test_output << _X("hostfxr_close failed: ") << std::hex << std::showbase << rcClose << std::endl;
 
     return rc == StatusCode::Success && rcClose == StatusCode::Success;
 }
index ea90961..020f601 100644 (file)
@@ -241,6 +241,27 @@ int main(const int argc, const pal::char_t *argv[])
         std::cout << tostr(test_output.str()).data() << std::endl;
         return success ? EXIT_SUCCESS : EXIT_FAILURE;
     }
+    else if (pal::strcmp(command, _X("run_app")) == 0)
+    {
+        // args: ... <hostfxr_path> <dotnet_command_line>
+        const int min_argc = 4;
+        if (argc < min_argc)
+        {
+            std::cerr << "Invalid arguments" << std::endl;
+            return -1;
+        }
+
+        const pal::string_t hostfxr_path = argv[2];
+
+        int remaining_argc = argc - min_argc + 1;
+        const pal::char_t **remaining_argv = &argv[min_argc - 1];
+
+        pal::stringstream_t test_output;
+        bool success =  host_context_test::app(host_context_test::check_properties::none, hostfxr_path, remaining_argc, remaining_argv, test_output);
+
+        std::cout << tostr(test_output.str()).data() << std::endl;
+        return success ? EXIT_SUCCESS : EXIT_FAILURE;
+    }
     else if (pal::strcmp(command, _X("resolve_component_dependencies")) == 0)
     {
         // args: ... <scenario> <hostfxr_path> <app_path> <component_path>
index a16cf7e..bd8b496 100644 (file)
@@ -16,7 +16,7 @@ namespace Component
         {
             componentCallCount++;
             entryPoint1CallCount++;
-            Console.WriteLine($"Called ComponentEntryPoint1(0x{arg.ToString("x")}, {size}) - component call count: {componentCallCount}");
+            Console.WriteLine($"Called {nameof(ComponentEntryPoint1)}(0x{arg.ToString("x")}, {size}) - component call count: {componentCallCount}");
             return entryPoint1CallCount;
         }
 
@@ -24,8 +24,15 @@ namespace Component
         {
             componentCallCount++;
             entryPoint2CallCount++;
-            Console.WriteLine($"Called ComponentEntryPoint2(0x{arg.ToString("x")}, {size}) - component call count: {componentCallCount}");
+            Console.WriteLine($"Called {nameof(ComponentEntryPoint2)}(0x{arg.ToString("x")}, {size}) - component call count: {componentCallCount}");
             return entryPoint2CallCount;
         }
+
+        public static int ThrowException(IntPtr arg, int size)
+        {
+            componentCallCount++;
+            Console.WriteLine($"Called {nameof(ThrowException)}(0x{arg.ToString("x")}, {size}) - component call count: {componentCallCount}");
+            throw new InvalidOperationException(nameof(ThrowException));
+        }
     }
 }
\ No newline at end of file
index a8b4405..4382f35 100644 (file)
@@ -82,6 +82,9 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
             public const int LibHostInvalidArgs = unchecked((int)0x80008092);
             public const int AppArgNotRunnable = unchecked((int)0x80008094);
             public const int FrameworkMissingFailure = unchecked((int)0x80008096);
+
+            public const int COMPlusException = unchecked((int)0xe0434352);
+            public const int SIGABRT = 134;
         }
     }
 }
diff --git a/src/installer/test/HostActivation.Tests/NativeHosting/ApplicationExecution.cs b/src/installer/test/HostActivation.Tests/NativeHosting/ApplicationExecution.cs
new file mode 100644 (file)
index 0000000..d5e6cf0
--- /dev/null
@@ -0,0 +1,117 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Microsoft.DotNet.Cli.Build.Framework;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices;
+using Xunit;
+
+namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting
+{
+    public partial class ApplicationExecution : IClassFixture<ApplicationExecution.SharedTestState>
+    {
+        private const string ApplicationExecutionArg = "run_app";
+
+        private readonly SharedTestState sharedState;
+
+        public ApplicationExecution(SharedTestState sharedTestState)
+        {
+            sharedState = sharedTestState;
+        }
+
+        [Fact]
+        public void RunApp()
+        {
+            var project = sharedState.PortableAppFixture.TestProject;
+            string[] args =
+            {
+                ApplicationExecutionArg,
+                sharedState.HostFxrPath,
+                project.AppDll
+            };
+
+            sharedState.CreateNativeHostCommand(args, sharedState.DotNetRoot)
+                .Execute()
+                .Should().Pass()
+                .And.InitializeContextForApp(project.AppDll)
+                .And.ExecuteApplication(sharedState.NativeHostPath, project.AppDll);
+        }
+
+        [Fact]
+        public void RunApp_UnhandledException()
+        {
+            var project = sharedState.PortableAppWithExceptionFixture.TestProject;
+            string[] args =
+            {
+                ApplicationExecutionArg,
+                sharedState.HostFxrPath,
+                project.AppDll
+            };
+
+            sharedState.CreateNativeHostCommand(args, sharedState.DotNetRoot)
+                .Execute()
+                .Should().Fail()
+                .And.InitializeContextForApp(project.AppDll)
+                .And.ExecuteApplicationWithException(sharedState.NativeHostPath, project.AppDll);
+        }
+
+        public class SharedTestState : SharedTestStateBase
+        {
+            public string HostFxrPath { get; }
+            public string DotNetRoot { get; }
+
+            public TestProjectFixture PortableAppFixture { get; }
+            public TestProjectFixture PortableAppWithExceptionFixture { get; }
+
+            public SharedTestState()
+            {
+                var dotNet = new Microsoft.DotNet.Cli.Build.DotNetCli(Path.Combine(TestArtifact.TestArtifactsPath, "sharedFrameworkPublish"));
+                DotNetRoot = dotNet.BinPath;
+                HostFxrPath = dotNet.GreatestVersionHostFxrFilePath;
+
+                PortableAppFixture = new TestProjectFixture("PortableApp", RepoDirectories)
+                    .EnsureRestored(RepoDirectories.CorehostPackages)
+                    .PublishProject();
+
+                PortableAppWithExceptionFixture = new TestProjectFixture("PortableAppWithException", RepoDirectories)
+                    .EnsureRestored(RepoDirectories.CorehostPackages)
+                    .PublishProject();
+            }
+
+            protected override void Dispose(bool disposing)
+            {
+                PortableAppFixture.Dispose();
+                PortableAppWithExceptionFixture.Dispose();
+
+                base.Dispose(disposing);
+            }
+        }
+    }
+
+    internal static class ApplicationExecutionResultExtensions
+    {
+        public static FluentAssertions.AndConstraint<CommandResultAssertions> ExecuteApplication(this CommandResultAssertions assertion, string hostPath, string appPath)
+        {
+            return assertion.HaveStdErrContaining($"Launch host: {hostPath}, app: {appPath}")
+                .And.HaveStdOutContaining("Hello World!");
+        }
+
+        public static FluentAssertions.AndConstraint<CommandResultAssertions> ExecuteApplicationWithException(this CommandResultAssertions assertion, string hostPath, string appPath)
+        {
+            var constraint = assertion.ExecuteApplication(hostPath, appPath);
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            {
+                return constraint.And.HaveStdOutContaining($"hostfxr_run_app threw exception: 0x{Constants.ErrorCode.COMPlusException.ToString("x")}");
+            }
+            else
+            {
+                // Exception is unhandled by native host on non-Windows systems
+                return constraint.And.ExitWith(Constants.ErrorCode.SIGABRT)
+                    .And.HaveStdErrContaining("Unhandled exception. System.Exception: Goodbye World!");
+            }
+        }
+    }
+}
index 27aa0c5..e123787 100644 (file)
@@ -6,6 +6,7 @@ using Microsoft.DotNet.Cli.Build.Framework;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Runtime.InteropServices;
 using Xunit;
 
 namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting
@@ -143,6 +144,28 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting
             }
         }
 
+        [Fact]
+        public void CallDelegate_UnhandledException()
+        {
+            string entryPoint = "ThrowException";
+            var componentProject = sharedState.ComponentWithNoDependenciesFixture.TestProject;
+            string[] args =
+            {
+                ComponentActivationArg,
+                sharedState.HostFxrPath,
+                componentProject.RuntimeConfigJson,
+                componentProject.AppDll,
+                sharedState.ComponentTypeName,
+                entryPoint,
+            };
+
+            sharedState.CreateNativeHostCommand(args, sharedState.DotNetRoot)
+                .Execute()
+                .Should().Fail()
+                .And.InitializeContextForConfig(componentProject.RuntimeConfigJson)
+                .And.ExecuteComponentEntryPointWithException(entryPoint, 1);
+        }
+
         public class SharedTestState : SharedTestStateBase
         {
             public string HostFxrPath { get; }
@@ -179,8 +202,28 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting
     {
         public static FluentAssertions.AndConstraint<CommandResultAssertions> ExecuteComponentEntryPoint(this CommandResultAssertions assertion, string methodName, int componentCallCount, int returnValue)
         {
-            return assertion.HaveStdOutContaining($"Called {methodName}(0xdeadbeef, 42) - component call count: {componentCallCount}")
+            return assertion.ExecuteComponentEntryPoint(methodName, componentCallCount)
                 .And.HaveStdOutContaining($"{methodName} delegate result: 0x{returnValue.ToString("x")}");
         }
+
+        public static FluentAssertions.AndConstraint<CommandResultAssertions> ExecuteComponentEntryPointWithException(this CommandResultAssertions assertion, string methodName, int componentCallCount)
+        {
+            var constraint = assertion.ExecuteComponentEntryPoint(methodName, componentCallCount);
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            {
+                return constraint.And.HaveStdOutContaining($"{methodName} delegate threw exception: 0x{Constants.ErrorCode.COMPlusException.ToString("x")}");
+            }
+            else
+            {
+                // Exception is unhandled by native host on non-Windows systems
+                return constraint.And.ExitWith(Constants.ErrorCode.SIGABRT)
+                    .And.HaveStdErrContaining($"Unhandled exception. System.InvalidOperationException: {methodName}");
+            }
+        }
+
+        public static FluentAssertions.AndConstraint<CommandResultAssertions> ExecuteComponentEntryPoint(this CommandResultAssertions assertion, string methodName, int componentCallCount)
+        {
+            return assertion.HaveStdOutContaining($"Called {methodName}(0xdeadbeef, 42) - component call count: {componentCallCount}");
+        }
     }
 }