diff --git a/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs b/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs index def1570be7975c..923ff3c00d4f92 100644 --- a/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs +++ b/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs @@ -104,17 +104,59 @@ private static bool HasCustomAttributeWithName(this MemberInfo memberInfo, strin #if NET return ctorInfo.Invoke(BindingFlags.DoNotWrapExceptions, null, parameters, null); #else - object? result = null; try { - result = ctorInfo.Invoke(parameters); + return ctorInfo.Invoke(parameters); } - catch (TargetInvocationException ex) + catch (TargetInvocationException ex) when (ex.InnerException is not null) { ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); + throw; // unreachable } +#endif + } - return result; + /// + /// Invokes without wrapping any exception thrown by the + /// target method in a . This matches the behavior of + /// the Reflection.Emit-based accessor, which emits direct calls into user code. + /// + public static object? InvokeNoWrapExceptions(this MethodInfo methodInfo, object? obj, object?[]? parameters) + { +#if NET + return methodInfo.Invoke(obj, BindingFlags.DoNotWrapExceptions, binder: null, parameters, culture: null); +#else + try + { + return methodInfo.Invoke(obj, parameters); + } + catch (TargetInvocationException ex) when (ex.InnerException is not null) + { + ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); + throw; // unreachable + } +#endif + } + + /// + /// Invokes without wrapping any exception thrown by the + /// constructor in a . This matches the behavior of + /// the Reflection.Emit-based accessor, which emits direct calls into user code. + /// + public static object InvokeNoWrapExceptions(this ConstructorInfo constructorInfo, object?[]? parameters) + { +#if NET + return constructorInfo.Invoke(BindingFlags.DoNotWrapExceptions, binder: null, parameters, culture: null); +#else + try + { + return constructorInfo.Invoke(parameters); + } + catch (TargetInvocationException ex) when (ex.InnerException is not null) + { + ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); + throw; // unreachable + } #endif } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/MemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/MemberAccessor.cs index 248a560cd58ce6..9fbe468cebcced 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/MemberAccessor.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/MemberAccessor.cs @@ -31,8 +31,13 @@ static MemberAccessor Initialize() { MemberAccessor value = #if NET - // if dynamic code isn't supported, fallback to reflection - RuntimeFeature.IsDynamicCodeSupported ? + // On platforms where dynamic code is supported but not compiled to native code + // (e.g. the Mono interpreter, WASM and iOS), the IL emitted by the Reflection.Emit + // based accessor is only interpreted, offering no throughput benefit over plain + // reflection while still pulling in the Reflection.Emit stack. Gating on + // IsDynamicCodeCompiled (rather than IsDynamicCodeSupported) keeps Reflection.Emit + // on JIT-backed runtimes but lets the trimmer remove it everywhere else. + RuntimeFeature.IsDynamicCodeCompiled ? new ReflectionEmitCachingMemberAccessor() : new ReflectionMemberAccessor(); #elif NETFRAMEWORK diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionMemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionMemberAccessor.cs index fd6f38ada0db7a..6cbe842f1085df 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionMemberAccessor.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionMemberAccessor.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reflection; +using System.Text.Json.Reflection; namespace System.Text.Json.Serialization.Metadata { @@ -33,7 +34,7 @@ public ReflectionMemberAccessor() : null; } - return () => ctorInfo.Invoke(null); + return () => ctorInfo.InvokeNoWrapExceptions(null); } public override Func CreateParameterizedConstructor(ConstructorInfo constructor) @@ -56,17 +57,10 @@ public override Func CreateParameterizedConstructor(ConstructorI argsToPass[i] = arguments[i]; } - try - { - return (T)constructor.Invoke(argsToPass); - } - catch (TargetInvocationException e) - { - // Plumb ArgumentException through for tuples with more than 7 generic parameters, e.g. - // System.ArgumentException : The last element of an eight element tuple must be a Tuple. - // This doesn't apply to the method below as it supports a max of 4 constructor params. - throw e.InnerException ?? e; - } + // Not wrapping in TargetInvocationException also plumbs ArgumentException through for + // tuples with more than 7 generic parameters, e.g. + // System.ArgumentException : The last element of an eight element tuple must be a Tuple. + return (T)constructor.InvokeNoWrapExceptions(argsToPass); }; } @@ -108,7 +102,7 @@ public override JsonTypeInfo.ParameterizedConstructorDelegate { - try - { - return (T)constructor.Invoke(new object?[] { value }); - } - catch (TargetInvocationException e) - { - throw e.InnerException ?? e; - } + return (T)constructor.InvokeNoWrapExceptions(new object?[] { value }); }; } @@ -143,7 +130,7 @@ public override JsonTypeInfo.ParameterizedConstructorDelegate CreatePropertyGetter(Property return delegate (object obj) { - return (TProperty)getMethodInfo.Invoke(obj, null)!; + return (TProperty)getMethodInfo.InvokeNoWrapExceptions(obj, null)!; }; } @@ -177,7 +164,7 @@ public override Func CreatePropertyGetter CreatePropertySetter(Proper return delegate (object obj, TProperty value) { - setMethodInfo.Invoke(obj, new object[] { value! }); + setMethodInfo.InvokeNoWrapExceptions(obj, new object[] { value! }); }; } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ExceptionTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ExceptionTests.cs index d9b76fee4da018..783db46afec111 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ExceptionTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ExceptionTests.cs @@ -3,8 +3,10 @@ using System.Collections.Generic; using System.Reflection; +using System.Runtime.CompilerServices; using System.Runtime.Serialization; using System.Text.Encodings.Web; +using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.Text.Json.Serialization.Tests @@ -632,5 +634,70 @@ public class PocoConverterThrowingCustomJsonException : JsonConverter throw new JsonException(ExceptionMessage, ExceptionPath, 0, 0); } + + [Fact] + public static void ThrowingMembers_PropagateOriginalException() + { + // Exercises the default member accessor (Reflection.Emit on runtimes that compile dynamic code). + AssertThrowingMembersPropagateOriginalException(); + } + + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public static void ThrowingMembers_PropagateOriginalException_WithReflectionMemberAccessor() + { + // Disabling dynamic code support routes serialization through the reflection-based member + // accessor (as happens on WASM, iOS and the Mono interpreter) instead of the Reflection.Emit + // accessor. Exceptions thrown by user getters, setters and constructors must still propagate + // unwrapped rather than being surfaced as a TargetInvocationException. + var options = new RemoteInvokeOptions + { + RuntimeConfigurationOptions = + { + ["System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported"] = false + } + }; + + RemoteExecutor.Invoke(static () => + { + Assert.False(RuntimeFeature.IsDynamicCodeCompiled); + AssertThrowingMembersPropagateOriginalException(); + }, options).Dispose(); + } + + private static void AssertThrowingMembersPropagateOriginalException() + { + InvalidOperationException ex; + + ex = Assert.Throws(() => JsonSerializer.Serialize(new ClassWithThrowingGetter())); + Assert.Equal(ThrowingMember_ExceptionMessage, ex.Message); + + ex = Assert.Throws(() => JsonSerializer.Deserialize("""{"Value":1}""")); + Assert.Equal(ThrowingMember_ExceptionMessage, ex.Message); + + ex = Assert.Throws(() => JsonSerializer.Deserialize("{}")); + Assert.Equal(ThrowingMember_ExceptionMessage, ex.Message); + } + + private const string ThrowingMember_ExceptionMessage = "Exception thrown from user code."; + + public class ClassWithThrowingGetter + { + public int Value => throw new InvalidOperationException(ThrowingMember_ExceptionMessage); + } + + public class ClassWithThrowingSetter + { + public int Value { get => 0; set => throw new InvalidOperationException(ThrowingMember_ExceptionMessage); } + } + + public class ClassWithThrowingConstructor + { + [JsonConstructor] + public ClassWithThrowingConstructor(int value) + => throw new InvalidOperationException(ThrowingMember_ExceptionMessage); + + public int Value { get; } + } } }