From efc85a4be07571a10acd96192792c33b0570a6e4 Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 19 Apr 2024 20:19:08 +0900 Subject: [PATCH 1/4] Support for exception handling from unmanaged caller --- src/WinRT.Runtime/ExceptionHelpers.cs | 23 +++++++++++ .../MatchingRefApiCompatBaseline.txt | 5 +++ src/cswinrt/code_writers.h | 41 +++++++++++++++---- 3 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 src/WinRT.Runtime/MatchingRefApiCompatBaseline.txt diff --git a/src/WinRT.Runtime/ExceptionHelpers.cs b/src/WinRT.Runtime/ExceptionHelpers.cs index 6f0f237521..224cfc7fab 100644 --- a/src/WinRT.Runtime/ExceptionHelpers.cs +++ b/src/WinRT.Runtime/ExceptionHelpers.cs @@ -98,6 +98,29 @@ private static bool Initialize() return true; } + /// + /// Unhandled WinRT server exception event. + /// + public static event EventHandler UnhandledWinRTServerException; + + public class UnhandledWinRTServerExceptionEventArgs : EventArgs + { + public Exception Exception { get; } + public bool Handled { get; set; } + + public UnhandledWinRTServerExceptionEventArgs(Exception exception) + { + Exception = exception; + } + } + + public static bool TryHandleWinRTServerException(object sender, Exception ex) + { + UnhandledWinRTServerExceptionEventArgs args = new UnhandledWinRTServerExceptionEventArgs(ex); + UnhandledWinRTServerException?.Invoke(sender, args); + return args.Handled; + } + public static void ThrowExceptionForHR(int hr) { if (hr < 0) diff --git a/src/WinRT.Runtime/MatchingRefApiCompatBaseline.txt b/src/WinRT.Runtime/MatchingRefApiCompatBaseline.txt new file mode 100644 index 0000000000..29e315de4d --- /dev/null +++ b/src/WinRT.Runtime/MatchingRefApiCompatBaseline.txt @@ -0,0 +1,5 @@ +MembersMustExist : Member 'public void WinRT.ExceptionHelpers.add_UnhandledWinRTServerException(System.EventHandler)' does not exist in the reference but it does exist in the implementation. +MembersMustExist : Member 'public void WinRT.ExceptionHelpers.remove_UnhandledWinRTServerException(System.EventHandler)' does not exist in the reference but it does exist in the implementation. +TypesMustExist : Type 'WinRT.ExceptionHelpers.UnhandledWinRTServerExceptionEventArgs' does not exist in the reference but it does exist in the implementation. +MembersMustExist : Member 'public System.Boolean WinRT.ExceptionHelpers.TryHandleWinRTServerException(System.Object, System.Exception)' does not exist in the reference but it does exist in the implementation. +Total Issues: 4 diff --git a/src/cswinrt/code_writers.h b/src/cswinrt/code_writers.h index 9ae6c48443..9d2e8b7e37 100644 --- a/src/cswinrt/code_writers.h +++ b/src/cswinrt/code_writers.h @@ -5242,7 +5242,7 @@ return eventSource.EventActions; return std::pair{ marshalers, managed_marshaler{} }; } - void write_managed_method_call(writer& w, method_signature signature, std::string invoke_expression_format) + void write_managed_method_call(writer& w, method_signature signature, std::string type_name, std::string invoke_expression_format) { auto generic_abi_types = get_generic_abi_types(w, signature); bool have_generic_params = std::find_if(generic_abi_types.begin(), generic_abi_types.end(), @@ -5255,6 +5255,7 @@ return eventSource.EventActions; w.write( R"(% % +% try { % @@ -5262,10 +5263,17 @@ try } catch (Exception __exception__) { +if (global::WinRT.ExceptionHelpers.TryHandleWinRTServerException(__this__, __exception__)) +{ +return 0; +} global::WinRT.ExceptionHelpers.SetErrorInfo(__exception__); return global::WinRT.ExceptionHelpers.GetHRForException(__exception__); } return 0;)", + [&](writer& w) { + w.write(R"(% __this__ = default;)", type_name); + }, [&](writer& w) { if (!return_sig) return; return_marshaler.write_local(w); @@ -5340,7 +5348,8 @@ private static unsafe int Do_Abi_%% bind(method), bind( signature, - w.write_temp("global::WinRT.ComWrappersSupport.FindObject<%>(%).%%", + type_name, + w.write_temp("(__this__ = global::WinRT.ComWrappersSupport.FindObject<%>(%)).%%", type_name, have_generic_params ? "new IntPtr(thisPtr)" : "thisPtr", method.Name(), @@ -5376,7 +5385,8 @@ private static unsafe int Do_Abi_%% bind(setter), bind( setter_sig, - w.write_temp("global::WinRT.ComWrappersSupport.FindObject<%>(%).% = %", + type_name, + w.write_temp("(__this__ = global::WinRT.ComWrappersSupport.FindObject<%>(%)).% = %", type_name, have_generic_params ? "new IntPtr(thisPtr)" : "thisPtr", prop.Name(), @@ -5406,7 +5416,8 @@ private static unsafe int Do_Abi_%% bind(getter), bind( getter_sig, - w.write_temp("global::WinRT.ComWrappersSupport.FindObject<%>(%).%%", + type_name, + w.write_temp("(__this__ = global::WinRT.ComWrappersSupport.FindObject<%>(%)).%%", type_name, have_generic_params ? "new IntPtr(thisPtr)" : "thisPtr", prop.Name(), @@ -5455,10 +5466,11 @@ private static global::System.Runtime.CompilerServices.ConditionalWeakTable<%, g % private static unsafe int Do_Abi_%% { +% __this = default; %% = default; try { -var __this = global::WinRT.ComWrappersSupport.FindObject<%>(thisPtr); +__this = global::WinRT.ComWrappersSupport.FindObject<%>(thisPtr); var __handler = %.FromAbi(%); %% = _%_TokenTables.GetOrCreateValue(__this).AddEventHandler(__handler); __this.% += __handler; @@ -5466,12 +5478,17 @@ return 0; } catch (Exception __ex) { +if (global::WinRT.ExceptionHelpers.TryHandleWinRTServerException(__this, __ex)) +{ +return 0; +} return __ex.HResult; } })", !settings.netstandard_compat && !generic_type ? "[UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })]" : "", get_vmethod_name(w, add_method.Parent(), add_method), bind(add_method), + type_name, settings.netstandard_compat ? "" : "*", add_handler_event_token_name, type_name, @@ -5486,9 +5503,10 @@ return __ex.HResult; % private static unsafe int Do_Abi_%% { +% __this = default; try { -var __this = global::WinRT.ComWrappersSupport.FindObject<%>(thisPtr); +__this = global::WinRT.ComWrappersSupport.FindObject<%>(thisPtr); if(__this != null && _%_TokenTables.TryGetValue(__this, out var __table) && __table.RemoveEventHandler(%, out var __handler)) { __this.% -= __handler; @@ -5497,6 +5515,10 @@ return 0; } catch (Exception __ex) { +if (global::WinRT.ExceptionHelpers.TryHandleWinRTServerException(__this, __ex)) +{ +return 0; +} return __ex.HResult; } })", @@ -5504,6 +5526,7 @@ return __ex.HResult; get_vmethod_name(w, remove_method.Parent(), remove_method), bind(remove_method), type_name, + type_name, evt.Name(), remove_handler_event_token_name, evt.Name()); @@ -7082,7 +7105,7 @@ public static Guid PIID = GuidGenerator.CreateIID(typeof(%));)", [&](writer& w) { if (settings.netstandard_compat) { - write_managed_method_call(w, signature, + write_managed_method_call(w, signature, type_name, w.write_temp(R"( global::WinRT.ComWrappersSupport.MarshalDelegateInvoke(%, (% invoke) => { @@ -7105,9 +7128,9 @@ global::WinRT.ComWrappersSupport.MarshalDelegateInvoke(%, (% invoke) => } else { - write_managed_method_call(w, signature, + write_managed_method_call(w, signature, type_name, w.write_temp(R"( -global::WinRT.ComWrappersSupport.FindObject<%>(%).Invoke(%) +(__this__ = global::WinRT.ComWrappersSupport.FindObject<%>(%)).Invoke(%) )", type_name, is_generic ? "new IntPtr(thisPtr)" : "thisPtr", From 9b635458d811e0d4eadf9ea345c40d54b9dcf877 Mon Sep 17 00:00:00 2001 From: Steve Date: Sat, 20 Apr 2024 02:14:29 +0900 Subject: [PATCH 2/4] hoist null check --- src/WinRT.Runtime/ExceptionHelpers.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/WinRT.Runtime/ExceptionHelpers.cs b/src/WinRT.Runtime/ExceptionHelpers.cs index 224cfc7fab..dbd28af7f4 100644 --- a/src/WinRT.Runtime/ExceptionHelpers.cs +++ b/src/WinRT.Runtime/ExceptionHelpers.cs @@ -116,9 +116,14 @@ public UnhandledWinRTServerExceptionEventArgs(Exception exception) public static bool TryHandleWinRTServerException(object sender, Exception ex) { - UnhandledWinRTServerExceptionEventArgs args = new UnhandledWinRTServerExceptionEventArgs(ex); - UnhandledWinRTServerException?.Invoke(sender, args); - return args.Handled; + if (UnhandledWinRTServerException != null) + { + UnhandledWinRTServerExceptionEventArgs args = new UnhandledWinRTServerExceptionEventArgs(ex); + UnhandledWinRTServerException.Invoke(sender, args); + return args.Handled; + } + + return false; } public static void ThrowExceptionForHR(int hr) From 9bbbba76e5df204792e0fdb58b71ba93360a587b Mon Sep 17 00:00:00 2001 From: Steve Date: Sat, 20 Apr 2024 02:30:01 +0900 Subject: [PATCH 3/4] Address code review feedback --- src/WinRT.Runtime/ExceptionHelpers.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/WinRT.Runtime/ExceptionHelpers.cs b/src/WinRT.Runtime/ExceptionHelpers.cs index dbd28af7f4..c5c2857bb8 100644 --- a/src/WinRT.Runtime/ExceptionHelpers.cs +++ b/src/WinRT.Runtime/ExceptionHelpers.cs @@ -116,13 +116,14 @@ public UnhandledWinRTServerExceptionEventArgs(Exception exception) public static bool TryHandleWinRTServerException(object sender, Exception ex) { - if (UnhandledWinRTServerException != null) - { + EventHandler handler = UnhandledWinRTServerException; + if (handler != null) + { UnhandledWinRTServerExceptionEventArgs args = new UnhandledWinRTServerExceptionEventArgs(ex); - UnhandledWinRTServerException.Invoke(sender, args); - return args.Handled; - } - + handler.Invoke(sender, args); + return args.Handled; + } + return false; } From 681292308b61f75b8ab04b56dc2411d865425fb0 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 26 Jun 2024 22:01:52 +0900 Subject: [PATCH 4/4] Address CR feedbacks --- src/WinRT.Runtime/ExceptionHelpers.cs | 20 +++++++++++++++---- .../MatchingRefApiCompatBaseline.txt | 2 +- src/cswinrt/code_writers.h | 19 +++--------------- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/WinRT.Runtime/ExceptionHelpers.cs b/src/WinRT.Runtime/ExceptionHelpers.cs index c5c2857bb8..4727c8a7f4 100644 --- a/src/WinRT.Runtime/ExceptionHelpers.cs +++ b/src/WinRT.Runtime/ExceptionHelpers.cs @@ -114,17 +114,29 @@ public UnhandledWinRTServerExceptionEventArgs(Exception exception) } } - public static bool TryHandleWinRTServerException(object sender, Exception ex) + public static int HandleWinRTServerException(object sender, Exception ex, bool dispatchExceptionToUnmanagedCode) { EventHandler handler = UnhandledWinRTServerException; + if (handler != null) { - UnhandledWinRTServerExceptionEventArgs args = new UnhandledWinRTServerExceptionEventArgs(ex); + UnhandledWinRTServerExceptionEventArgs args = new(ex); handler.Invoke(sender, args); - return args.Handled; + if (args.Handled) + { + return 0; + } } - return false; + if (dispatchExceptionToUnmanagedCode) + { + SetErrorInfo(ex); + return GetHRForException(ex); + } + else + { + return ex.HResult; + } } public static void ThrowExceptionForHR(int hr) diff --git a/src/WinRT.Runtime/MatchingRefApiCompatBaseline.txt b/src/WinRT.Runtime/MatchingRefApiCompatBaseline.txt index 29e315de4d..0493e23d1a 100644 --- a/src/WinRT.Runtime/MatchingRefApiCompatBaseline.txt +++ b/src/WinRT.Runtime/MatchingRefApiCompatBaseline.txt @@ -1,5 +1,5 @@ MembersMustExist : Member 'public void WinRT.ExceptionHelpers.add_UnhandledWinRTServerException(System.EventHandler)' does not exist in the reference but it does exist in the implementation. MembersMustExist : Member 'public void WinRT.ExceptionHelpers.remove_UnhandledWinRTServerException(System.EventHandler)' does not exist in the reference but it does exist in the implementation. TypesMustExist : Type 'WinRT.ExceptionHelpers.UnhandledWinRTServerExceptionEventArgs' does not exist in the reference but it does exist in the implementation. -MembersMustExist : Member 'public System.Boolean WinRT.ExceptionHelpers.TryHandleWinRTServerException(System.Object, System.Exception)' does not exist in the reference but it does exist in the implementation. +MembersMustExist : Member 'public System.Int32 WinRT.ExceptionHelpers.HandleWinRTServerException(System.Object, System.Exception, System.Boolean)' does not exist in the reference but it does exist in the implementation. Total Issues: 4 diff --git a/src/cswinrt/code_writers.h b/src/cswinrt/code_writers.h index 9d2e8b7e37..dc280a9bcb 100644 --- a/src/cswinrt/code_writers.h +++ b/src/cswinrt/code_writers.h @@ -5263,12 +5263,7 @@ try } catch (Exception __exception__) { -if (global::WinRT.ExceptionHelpers.TryHandleWinRTServerException(__this__, __exception__)) -{ -return 0; -} -global::WinRT.ExceptionHelpers.SetErrorInfo(__exception__); -return global::WinRT.ExceptionHelpers.GetHRForException(__exception__); +return global::WinRT.ExceptionHelpers.HandleWinRTServerException(__this__, __exception__, true); } return 0;)", [&](writer& w) { @@ -5478,11 +5473,7 @@ return 0; } catch (Exception __ex) { -if (global::WinRT.ExceptionHelpers.TryHandleWinRTServerException(__this, __ex)) -{ -return 0; -} -return __ex.HResult; +return global::WinRT.ExceptionHelpers.TryHandleWinRTServerException(__this, __ex, false); } })", !settings.netstandard_compat && !generic_type ? "[UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })]" : "", @@ -5515,11 +5506,7 @@ return 0; } catch (Exception __ex) { -if (global::WinRT.ExceptionHelpers.TryHandleWinRTServerException(__this, __ex)) -{ -return 0; -} -return __ex.HResult; +return global::WinRT.ExceptionHelpers.TryHandleWinRTServerException(__this, __ex, false); } })", !settings.netstandard_compat && !generic_type ? "[UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })]" : "",