Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deserialization of generic types fails due to inability to find property setters that use init #1134

Closed
AArnott opened this issue Nov 22, 2020 · 13 comments
Assignees
Labels
Milestone

Comments

@AArnott
Copy link
Collaborator

AArnott commented Nov 22, 2020

Bug description

A generic type containing properties with an init accessor fails to serialize.

Repro steps

This type fails to serialize with the DynamicObjectResolver:

        [MessagePackObject]
        public class GenericPerson<T>
        {
            [Key(0)]
            public string Name { get; init; }
        }

This test fails:

        [Fact]
        public void RoundtripGenericClass()
        {
            var person = new GenericPerson<int> { Name = "bob" };
            byte[] msgpack = MessagePackSerializer.Serialize(person, MessagePackSerializerOptions.Standard);
            var deserialized = MessagePackSerializer.Deserialize<GenericPerson<int>>(msgpack, MessagePackSerializerOptions.Standard);
            Assert.Equal(person.Name, deserialized.Name);
        }

Expected behavior

Successful pass.

Actual behavior

  Message: 
    MessagePack.MessagePackSerializationException : Failed to deserialize MessagePack.Tests.DynamicObjectResolverTests+GenericPerson`1[[System.Int32, System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]] value.
    ---- System.MissingMethodException : Method not found: 'Void GenericPerson`1.set_Data(!0)'.
  Stack Trace: 
    MessagePackSerializer.Deserialize[T](MessagePackReader& reader, MessagePackSerializerOptions options) line 251
    MessagePackSerializer.Deserialize[T](ReadOnlyMemory`1 buffer, MessagePackSerializerOptions options, CancellationToken cancellationToken) line 270
    DynamicObjectResolverTests.RoundtripGenericClass() line 321
    ----- Inner Stack Trace -----
       at MessagePack.Formatters.MessagePack_Tests_DynamicObjectResolverTests\.GenericPerson`1\[\[System_Int32\, System_Private_CoreLib\]\]Formatter1.Deserialize(MessagePackReader& reader, MessagePackSerializerOptions options)
    MessagePackSerializer.Deserialize[T](MessagePackReader& reader, MessagePackSerializerOptions options) line 246
@AArnott AArnott added the bug label Nov 22, 2020
@AArnott AArnott self-assigned this Nov 22, 2020
@AArnott AArnott changed the title Serialization of generic types fails due to inability to find property setters that use init Deserialization of generic types fails due to inability to find property setters that use init Nov 22, 2020
AArnott added a commit to AArnott/MessagePack-CSharp that referenced this issue Nov 23, 2020
@AArnott
Copy link
Collaborator Author

AArnott commented Nov 23, 2020

Based on this doc it seems likely that our IL code gen has to be updated to include modreq([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) in the called method signature somehow. It seems all C# compiler generated calls to init methods require this, and it's evident in decompilers but not in our code-generated calls to it.

As to why it only fails in generic classes may be a bug/detail of the .NET 5 runtime.

@AArnott
Copy link
Collaborator Author

AArnott commented Nov 23, 2020

See linked dotnet/runtime bug for more info. I'm pretty certain this is a bug in .NET.

@AArnott
Copy link
Collaborator Author

AArnott commented Nov 23, 2020

This is a bug in .NET. .NET 6.0 has the fix, which may be ported to 5.x.
For now, a workaround is to use DynamicMethod to make the call instead. That translates in MessagePack to folks using the StandardResolverAllowPrivate resolver instead of StandardResolver.

@AArnott AArnott added this to the v2.3 milestone Nov 23, 2020
AArnott added a commit to AArnott/MessagePack-CSharp that referenced this issue Nov 23, 2020
…y setters

Fixes MessagePack-CSharp#1134 as much as we can while .NET has the underlying bug.
When .NET 6 ships with the fix, we can add a .NET 6 target that re-allows setting `init` property setters from the `DynamicObjectResolver`.
AArnott added a commit to AArnott/MessagePack-CSharp that referenced this issue Nov 25, 2020
…y setters

Fixes MessagePack-CSharp#1134 as much as we can while .NET has the underlying bug.
When .NET 6 ships with the fix, we can add a .NET 6 target that re-allows setting `init` property setters from the `DynamicObjectResolver`.
@simonziegler
Copy link

@AArnott thanks for moving #1147 to here. I probably cannot use StandardResolverAllowPrivate because of side effects through my code base. For now I am selectively using set; rather than init;. To help me chart the best path forwards pending .NET 6, can you let me know if you expect some sort of resolution in MessagePack, and if so how long you think it may take before that resolution is in place? I'll then orientate my medium term serialization strategy around that advice. Thanks!

@AArnott
Copy link
Collaborator Author

AArnott commented Dec 20, 2020

I'd like to make it "just work" with either dynamic resolver, perhaps by falling back to reflection when the bug is present. Hopefully by end of year.

@AArnott AArnott closed this as completed Jan 29, 2021
@simonziegler
Copy link

Hi @AArnott - since you closed this, what is the status pls?

@AArnott
Copy link
Collaborator Author

AArnott commented Jan 31, 2021

The PR with the fix has merged and will be included in our 2.3 release.
You can try it out prior to its release on nuget by following these instructions.

@simonziegler
Copy link

Thanks @AArnott. I just returned to working to integrate MessagePack into my data model. I see that 2.3 is in Alpha. Do you anticipate any issues for me using this release?

Many thanks, Simon

@AArnott
Copy link
Collaborator Author

AArnott commented Apr 14, 2021

@simonziegler no, there's no known instability in messagepack 2.3-alpha.

@simonziegler
Copy link

Hey @AArnott I was going to drop by here this week to let you know that 2.3-alpha is working great and without any issues regarding init/set. It all just works. Many thanks!

@SapiensAnatis
Copy link

SapiensAnatis commented Sep 17, 2022

Hey, I see this has been closed and a fix was included in 2.3, but I am on 2.4.35 and I just got an exception linking me to this issue while using the ContractlessStandardResolver (that's supposed to include the dynamic resolver, right?):

InitAccessorInGenericClassNotSupportedException: `init` property accessor [...] data_headers found in generic type, which is not supported with the DynamicObjectResolver. Use the AllowPrivate variety of the resolver instead. See https://github.com/neuecc/MessagePack-CSharp/issues/1134 for details.

I am having some trouble putting together a minimum reproducible example though, and my existing code probably wouldn't be much help because it's in an ASP.NET Core MVC app. I was able to fix it by including the [MessagePackObject(true)] decorator above my class, but I didn't think I would have to do that with the contractless resolver.

Is the ContractlessStandardResolver supposed to throw this error?

@AArnott
Copy link
Collaborator Author

AArnott commented Sep 20, 2022

@SapiensAnatis did you notice my comment #1134 (comment)? What runtime are you on? Anything less than .NET 6 may give you trouble unless you use an AllowPrivate resolver. But contractless+allowprivate is just terrible (it serializes fields and the properties that expose them).

@SapiensAnatis
Copy link

SapiensAnatis commented Sep 24, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants