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

MessagePackFormatter(typeof(TypelessFormatter)) throws exception during Serialization #1097

Closed
gjaw opened this issue Oct 28, 2020 · 6 comments · Fixed by #1100
Closed

MessagePackFormatter(typeof(TypelessFormatter)) throws exception during Serialization #1097

gjaw opened this issue Oct 28, 2020 · 6 comments · Fixed by #1100
Assignees
Labels
Milestone

Comments

@gjaw
Copy link

gjaw commented Oct 28, 2020

Bug description

Context: I want to use TypelessFormatter only for a specific member of a class instance, while using standard formatter (or other formatters) for everything else.

The example for this given in README.md is:

public class Foo
{
    // use Typeless(this field only)
    [MessagePackFormatter(typeof(TypelessFormatter))]
    public object Bar;
}

Trying to serialize such a class Foo results in MissingMethodException: Constructor on type 'MessagePack.Formatters.TypelessFormatter' not found.

This is kind of expected since all of the resolvers I saw will create an instance of the found formatter, but TypelessFormatter only has a private constructor as it apparently should only be accessed through its singleton Instance property.

Repro steps

Here's a fully self-contained code to repro and showing my final intent:

using System;
using System.IO;
using MessagePack;
using MessagePack.Formatters;
using MessagePack.Resolvers;

namespace BugTest
{
    public class Program
    {
        private static MessagePackSerializerOptions serializerOptions = MessagePackSerializerOptions.Standard
                                                                                .WithResolver(CompositeResolver.Create(
                                                                                    new[] { TypelessFormatter.Instance },
                                                                                    new IFormatterResolver[] { StandardResolver.Instance }));
        // alternative options I tried:
        private static MessagePackSerializerOptions serializerOptions1 = TypelessContractlessStandardResolver.Options;

        public static void Main(string[] args)
        {
            double[] a = { 1, 2 };
            int[] b = { 3, 4 };
            object[] test_args = new object[] { a, b };
            var test_source = new SerTest(test_args);

            var bs = ToByteArraySingle(test_source);
            var test_destination = FromByteArraySingle<SerTest>(bs);

            if (test_destination.Args[0] is double[] a2)
            {
                // All good, TypelessFormatter was able to get the inner types right
                Console.Write("All ok");
            }
            else
            {
                throw new InvalidOperationException($"Args[0] is of type {test_destination.Args[0].GetType().Name}");
            }
        }

        public static byte[] ToByteArraySingle<T>(T obj)
        {
            using var ms = new MemoryStream();
            MessagePackSerializer.Serialize(ms, obj, serializerOptions); // Exception is thrown here
            return ms.ToArray();
        }

        public static T FromByteArraySingle<T>(byte[] data)
        {
            return MessagePackSerializer.Deserialize<T>(data, serializerOptions);
        }
    }

    [MessagePackObject]
    public class SerTest
    {
        [Key(0)]
        [MessagePackFormatter(typeof(TypelessFormatter))]
        public object[] Args { get; }

        public SerTest(object[] args)
        {
            Args = args;
        }
    }
}

Expected behavior

As per the README, I would expect the TypelessFormatter to be successfully used for the property with [MessagePackFormatter(typeof(TypelessFormatter))] attribute applied.

Actual behavior

An exception MissingMethodException: Constructor on type 'MessagePack.Formatters.TypelessFormatter' not found. is thrown.

  • Version used: Commit 896ed3c (HEAD of master branch at the time of writing this bug report)
  • Runtime: .NET Core 3.1

Additional context

Not sure whether this is a bug, as in the example should work, or if this is a documentation bug, as in the example shouldn't even work but is included in README by mistake.

@neuecc
Copy link
Member

neuecc commented Oct 29, 2020

I think this is a bug.
In v1, it could be created via ctor, and the fields for all the caches were static and shared.
https://github.com/neuecc/MessagePack-CSharp/blob/v1.9.11/src/MessagePack/Formatters/TypelessFormatter.cs
I think v2 should go back to that way as well. @AArnott

@gjaw
Copy link
Author

gjaw commented Oct 29, 2020

Moreover, I noticed that currently there is also a ForceTypelessFormatter<T> which is just a generics-typed warpper around the singleton TypelessFormatter. I know that it's used internally, but is marked public. It's missing from the exposed API in the NuGet version I was using in the original project, but I happened to implement my own very similar wrapper just to see if that would be an adequate workaround. However, using that one leads to stack overflow with Object[] typed values as TypelessFormatter.Serialize is still asking the resolver to give the formatter to call Serialize on, and it of course gives the attribute-forced ForceTypelessFormatter<Object[]>, leading to infinite recursion.

@AArnott
Copy link
Collaborator

AArnott commented Oct 30, 2020

is marked public. It's missing from the exposed API in the NuGet version

ForceTypelessFormatter<T> add added very recently in #1078. It probably should have merged into develop for v2.3 instead of master since it adds new public API. But anyway, it's not on nuget yet because we haven't shipped from master since that PR was merged.

leading to infinite recursion

Ya, I still think we need to support resolver delegation to avoid this problem. I just haven't had time to design that.

@gjaw do you know that you can use Typeless as your top-level resolver and it only uses typeless when the concrete type may not be known at deserialization time? At least, I think that's how it behaves. So for now perhaps you can just use the Typeless serializer/resolver to workaround the failure to use the attribute.

the fields for all the caches were static and shared

@neuecc I don't think this is of concern in v2.x because all those mutable fields have moved to MessagePackSerializationOptions. So they can be controlled in other ways.

In v1, it could be created via ctor

That's my bad. I didn't realize that formatters needed public default ctors for the attribute scenario. I'll add a test and fix the bug.

@AArnott AArnott self-assigned this Oct 30, 2020
@AArnott AArnott added the bug label Oct 30, 2020
@AArnott AArnott added this to the v2.2 milestone Oct 30, 2020
AArnott added a commit to AArnott/MessagePack-CSharp that referenced this issue Oct 30, 2020
This corrects a regression from 1.x to 2.0.

Fixes MessagePack-CSharp#1097
AArnott added a commit to AArnott/MessagePack-CSharp that referenced this issue Oct 30, 2020
This corrects a regression from 1.x to 2.0.

Fixes MessagePack-CSharp#1097
@gjaw
Copy link
Author

gjaw commented Oct 30, 2020

do you know that you can use Typeless as your top-level resolver and it only uses typeless when the concrete type may not be known at deserialization time? At least, I think that's how it behaves. So for now perhaps you can just use the Typeless serializer/resolver to workaround the failure to use the attribute.

Thanks, and yes. Using Typeless resolver did work for that case, but since the code did use unknown "object" types elsewhere in custom serialization relying on the lack of Typeless formatter, adding the Typeless resolver did cause other parts of the code to break. But for your peace of mind, I was able to work around those kinks so this bug didn't become a blocker.

Anyway, the PR also looks good to me for this bug specifically, but I'm afraid that it might not allow the example case of serializing object[] to work and would still need the delegation work to prevent infinite recursion. Or did I miss a subtle detail?

@AArnott
Copy link
Collaborator

AArnott commented Oct 31, 2020

but I'm afraid that it might not allow the example case of...

Please open a separate bug for that.

@gjaw
Copy link
Author

gjaw commented Oct 31, 2020

Please open a separate bug for that.

With the merged changes I cannot reproduce infinite recursion. Using the attribute on a object type works fine now. And perhaps I imagined it wrong, but README actually uses pure object type, not object[]. So for that example it does work right.

And now I realize that the MessagePackFormatter attribute only applies to the very exact item that must be of declared type object. If that object itself contains any other object type items inside it that do get serialized, the typeless formatter will not be used for them. So really, for my very original task, the "only" correct solution is to use the Typeless resolver. Learning by reporting :)

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

Successfully merging a pull request may close this issue.

3 participants