Replace Unsafe.Unbox with StrongBox for JSON source-generated struct accessors by jkoritzinsky · Pull Request #133997 · dotnet/runtime

jkoritzinsky · 2026-09-15T22:28:47Z

Description

This change replaces usages of Unsafe.Unbox in the System.Text.Json source generator with StrongBox for struct member getters and setters during deserialization and serialization.

Key changes:

Updated JsonSourceGenerator.Emitter.cs so value-type property setters, getters, [UnsafeAccessor]s, and reflection fallbacks use StrongBox instead of Unsafe.Unbox.
Configured source-generated value-type JsonTypeInfo instances to instantiate their target object wrapped in StrongBox.
Updated ObjectDefaultConverter and ObjectWithParameterizedConstructorConverter to handle StrongBox during property population and unwrap box.Value upon completing deserialization.

Note

This pull request was created with GitHub Copilot.

…accessors Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

azure-pipelines · 2026-09-15T22:29:30Z

Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

dotnet-policy-service · 2026-09-15T22:29:54Z

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

Copilot

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 3 · 2

Open (5)

Keep customized struct creators StrongBox-compatible · New
Wrap pre-populated parameterized structs before generated setters · New
Do not expose StrongBox through JsonTypeInfo.CreateObject · New
Preserve struct mutations made by deserialization callbacks · New
Preserve callbacks for parameterized source-generated structs · New

What changed in this PR

This PR replaces Unsafe.Unbox<T> with StrongBox<T> for source-generated struct deserialization accessors.

Changes:

Adds StrongBox-aware generated getters, setters, and reflection fallbacks.
Wraps source-generated value-type instances during deserialization.
Updates object converters and compilation references.

File	Description
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/CompilationHelper.cs	Updated as part of this pull request.
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs	Updated as part of this pull request.
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs	Updated as part of this pull request.
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.Helpers.cs	Updated as part of this pull request.
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs	Updated as part of this pull request.
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs	Updated as part of this pull request.
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs	Updated as part of this pull request.

eiriktsarpalis · 2026-09-16T09:17:43Z

                {
                    return typeGenerationSpec.TypeRef.IsValueType
-                        ? $"""static (obj, value) => {GetUnboxExpression(contextSpec, declaringTypeFQN)}.{propertyName} = value!"""
+                        ? $"""static (obj, value) => (({StrongBoxTypeRef}<{declaringTypeFQN}>)obj).Value.{propertyName} = value!"""


I concur, this is a breaking change unfortunately. Users can access, modify, or wrap the delegate directly via the JsonPropertyInfo.Set property and anybody doing so today is relying on the current implicit contract.

Addressed in 8a9c9c0 without changing JsonPropertyInfo.Get/.Set's existing contract or adding new public API.

JsonPropertyInfo<T> now lazily detects, per property, whether the assigned Get/Set delegate understands a StrongBox<TDeclaringType>-wrapped obj: it tries the delegate directly first, and on InvalidCastException falls back to a legacy unbox/invoke/rebox path (using the pre-existing public System.Runtime.CompilerServices.IStrongBox interface, which StrongBox<T> implements). The result is cached per property so the fallback only triggers once. This transparently covers:

source-gen assemblies built before StrongBox-based struct accessors existed, and

any resolver-modifier-supplied delegate written against the historical "obj is a boxed TDeclaringType" contract (the exact scenario you flagged) - including one captured/wrapped from JsonPropertyInfo.Get/.Set and reassigned.

Since the public untyped Get/Set properties must keep returning the exact delegate instance a caller assigned (there's an existing test enforcing that), the compatibility handling lives behind the internal typed accessor instead, and the two internal call sites that previously invoked the untyped Get/Set directly (TryGetPrePopulatedValue, extension-data population) now go through new GetValueAsObject/SetValueAsObject methods so they get the same handling.

No emitter changes were needed - new-gen's StrongBox-aware delegates always succeed on the fast path.

Note
This reply was generated with AI assistance.

_{Auto-replied by the GitHub Copilot app}

This still breaks callers of the JsonPropertyInfo.Set delegate populated by the source generator. For example, this console app uses a contract modifier to trim string properties after deserialization. It invokes the generated Get/Set delegates without replacing them:

using System.Text.Json; using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; JsonSerializerOptions options = new() { TypeInfoResolver = AppJsonContext.Default.WithAddedModifier(TrimStringProperties) }; Contact contact = JsonSerializer.Deserialize<Contact>( """{"Name":" Alice ","Email":" alice@example.com "}""", options); Console.WriteLine($"Name: '{contact.Name}'"); Console.WriteLine($"Email: '{contact.Email}'"); static void TrimStringProperties(JsonTypeInfo typeInfo) { if (typeInfo.Kind is not JsonTypeInfoKind.Object) { return; } JsonPropertyInfo[] properties = typeInfo.Properties .Where(p => p.PropertyType == typeof(string) && p.Get is not null && p.Set is not null) .ToArray(); Action<object>? originalCallback = typeInfo.OnDeserialized; typeInfo.OnDeserialized = obj => { originalCallback?.Invoke(obj); foreach (JsonPropertyInfo property in properties) { if (property.Get!(obj) is string value) { property.Set!(obj, value.Trim()); } } }; } public struct Contact { public string? Name { get; set; } public string? Email { get; set; } } [JsonSerializable(typeof(Contact))] internal partial class AppJsonContext : JsonSerializerContext;

Before the change, this prints:

Name: 'Alice' Email: 'alice@example.com'

After rebuilding with the generator and library from 8a9c9c0, it throws at property.Set!(obj, value.Trim()):

System.InvalidCastException: Unable to cast object of type 'Contact' to type 'System.Runtime.CompilerServices.StrongBox`1[Contact]'.

I don't think this would be possible to fix without keeping our dependency on Unsafe.Unbox.

Yes, it looks like we're going to need new APIs here. @agocke mentioned that we were looking a new API where we the gesture to use the source generator is on the user's type. I think the fixes to remove Unsafe.Unbox (and UnsafeAccessor so we can remove any need for AllowUnsafeBlocks) will only be possible on such an API. Trying to hack it here is likely a dead end after my investigation.

Sorry, which API was that? The only potential solution I'm aware of is this language proposal. Another possible near-term hack is exposing an EB.Never method in STJ.dll that proxies Unsafe.Unbox although I find that to be a bit unsavory.

@agocke mentioned that you had a prototype where you put the JsonSerializable attribute (or another attribute) on the type that can be serialized instead of on the context type. With that model, we could generate code into the user type that could access the private fields/properties and would avoid the use cases for Unsafe.Unbox and UnsafeAcessor today (and enable the JSON source generator to never need AllowUnsafeBlocks=true in the csproj for the new UX gesture).

Unsafe.Unbox and unsafe accessor use are completely orthogonal here. The former is used pretty much everywhere you need to access a setter on a struct, so accessibility doesn't play a role.

…mpiler warnings - Ensure JsonTypeInfo<T>.CreateObject continues returning boxed T instead of StrongBox<T> - Wrap CreateObject return values and pre-populated values in StrongBox<T> in object converters - Support custom CreateObject modifier delegates for source-generated structs - Preserve struct mutations in deserialization callbacks by unboxing and copying back - Add tests for CreateObject return types, custom modifiers, and struct callbacks Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 3

Open (3)

Do not expose StrongBox to custom struct property delegates · New
Gate StrongBox handling to StrongBox-aware generated metadata · New
Keep customized struct creators StrongBox-compatible

Resolved since last review (4)

Do not expose StrongBox through JsonTypeInfo.CreateObject
Wrap pre-populated parameterized structs before generated setters
Preserve callbacks for parameterized source-generated structs
Preserve struct mutations made by deserialization callbacks

+                    obj = IsValueType && jsonTypeInfo.IsSourceGenerated && state.Current.ReturnValue is not StrongBox<T>
+                        ? new StrongBox<T>((T)state.Current.ReturnValue!)
+                        : state.Current.ReturnValue!;


        private static JsonTypeInfo<T> CreateCore<T>(JsonConverter converter, JsonSerializerOptions options)
        {
            var typeInfo = new JsonTypeInfo<T>(converter, options);
+            typeInfo.IsSourceGenerated = true;


Addresses review feedback that generated struct property setters hard-casting 'obj' to StrongBox<T> is an undocumented breaking change to the historic JsonPropertyInfo.Get/.Set contract, where 'obj' was always a plain boxed T. JsonPropertyInfo<T> now lazily detects, per property, whether the assigned Get/Set delegate understands a StrongBox<TDeclaringType>-wrapped 'obj' by trying it directly first and falling back to a legacy unbox/invoke/rebox path (via the pre-existing public IStrongBox interface) on InvalidCastException. The result is cached so the fallback triggers only once per property. This covers source-gen assemblies built before StrongBox-based struct accessors existed, and any resolver-modifier-supplied delegate written against the historical 'obj is a boxed TDeclaringType' contract - all without changing the emitter or adding new public API. The public untyped Get/Set properties must keep returning the exact delegate instance a caller assigned (an existing, tested contract), so the StrongBox-compatibility handling is applied only to the internal typed Get/Set path, and internal call sites that previously invoked the untyped Get/Set directly (TryGetPrePopulatedValue, extension-data population) are routed through new GetValueAsObject/SetValueAsObject methods instead. Also fixes a pre-existing test bug (JSON property name casing mismatch) that was masking a lack of runtime coverage for parameterized-constructor struct callback mutations, and adds a new regression test simulating a legacy-style (pre-StrongBox) resolver-modifier-supplied Get/Set pair. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

Two unresolved moderate issues remain in JsonPropertyInfoOfT.cs, including a possible serialization InvalidCastException and hot-path overhead.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 · 2

Open (4)

Gate StrongBox handling to StrongBox-aware generated metadata
Do not expose StrongBox to custom struct property delegates
Avoid an extra delegate hop for every property access · New
Do not cache the legacy getter path for boxed serialization · New

Resolved since last review (1)

Keep customized struct creators StrongBox-compatible

+            _typedGet = obj => InvokeGetter(rawGetter, obj);
+            _untypedGet = getter is Func<object, object?> untypedGetter ? untypedGetter : obj => rawGetter(obj);


+            if (_getterRequiresLegacyUnbox)
            {
-                _typedSet = typedSetter;
-                _untypedSet = setter is Action<object, object?> untypedSet ? untypedSet : (obj, value) => typedSetter(obj, (T)value!);
+                return rawGetter(((IStrongBox)obj).Value!);
            }


MichalPetryka · 2026-09-17T14:42:19Z

Does the new runtime handling here work with data generated using older generator versions?

eiriktsarpalis · 2026-09-17T15:33:20Z

Does the new runtime handling here work with data generated using older generator versions?

That's a great point, I expect this would break as well (and is less contrived than the scenario I gave in #133997 (comment))

Replace Unsafe.Unbox with StrongBox for JSON source-generated struct …

d609dfd

…accessors Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings September 15, 2026 22:28

github-actions Bot added the area-System.Text.Json label Sep 15, 2026

dotnet-policy-service Bot assigned jkoritzinsky Sep 15, 2026

github-project-automation Bot added this to AppModel Sep 15, 2026

Copilot started reviewing on behalf of jkoritzinsky September 15, 2026 22:29 View session

jkoritzinsky added the reduce-unsafe label Sep 15, 2026

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 15, 2026 23:27

Copilot started reviewing on behalf of jkoritzinsky September 15, 2026 23:27 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 16, 2026 20:56

Copilot started reviewing on behalf of jkoritzinsky September 16, 2026 20:56 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

		_typedGet = obj => InvokeGetter(rawGetter, obj);
		_untypedGet = getter is Func<object, object?> untypedGetter ? untypedGetter : obj => rawGetter(obj);

Conversation

jkoritzinsky commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

azure-pipelines Bot commented Sep 15, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

eiriktsarpalis Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

jkoritzinsky Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

eiriktsarpalis Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

jkoritzinsky Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

eiriktsarpalis Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkoritzinsky Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

eiriktsarpalis Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

MichalPetryka commented Sep 17, 2026

Uh oh!

eiriktsarpalis commented Sep 17, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jkoritzinsky commented Sep 15, 2026 •

edited

Loading

eiriktsarpalis Sep 24, 2026 •

edited

Loading