From b75d96939b76b2bf0a4e4693e5d9d22c517b0e39 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 10 Jul 2026 19:48:33 +0300 Subject: [PATCH] Select STJ member accessor based on IsDynamicCodeCompiled MemberAccessor selected the Reflection.Emit-based accessor whenever RuntimeFeature.IsDynamicCodeSupported was true. On platforms where dynamic code is supported but only interpreted (the Mono interpreter, WASM, iOS), the emitted IL is interpreted anyway, so Reflection.Emit provides no throughput benefit while keeping the ~50KB Reflection.Emit stack un-trimmable. Gate on IsDynamicCodeCompiled instead so the trimmer removes Reflection.Emit on those platforms, matching the pattern already used by DependencyInjection and Regex. JIT-backed runtimes and NativeAOT are unaffected. This routes WASM/iOS to ReflectionMemberAccessor, which invoked user getters, setters and constructors via MethodInfo/ConstructorInfo.Invoke and wrapped their exceptions in TargetInvocationException, unlike the Reflection.Emit accessor which emits direct calls. That was already a latent bug on NativeAOT. Add InvokeNoWrapExceptions helpers (BindingFlags.DoNotWrapExceptions on .NET, an ExceptionDispatchInfo polyfill downlevel) and route all user-code invoke sites through them so exceptions propagate unwrapped on every accessor. Fixes #38693. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/System/ReflectionExtensions.cs | 50 ++++++++++++-- .../Serialization/Metadata/MemberAccessor.cs | 9 ++- .../Metadata/ReflectionMemberAccessor.cs | 37 ++++------ .../Serialization/ExceptionTests.cs | 67 +++++++++++++++++++ 4 files changed, 132 insertions(+), 31 deletions(-) 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; } + } } }