msgpack / msgpack-cli

MessagePack implementation for Common Language Infrastructure / msgpack.org[C#]
http://msgpack.org
Apache License 2.0
834 stars 175 forks source link

Guid and List<T> in same class = nasty crash #207

Closed niqhtei closed 7 years ago

niqhtei commented 7 years ago

When the DTO has both Guid and List as properties, serializer only serializes the List<> and omits Guid property completely. Trying to de-serialize such data results in a nasty nasty exception:

Managed Debugging Assistant 'FatalExecutionEngineError' has detected a problem in C:\blabla\bin\Debug\Serializers.vshost.exe.

Additional information: The runtime has encountered a fatal error. The address of the error was at 0xdf24bc2a, on thread 0x3a40. The error code is 0xc0000005. This error may be a bug in the CLR or in the unsafe or non-verifiable portions of user code. Common sources of this bug include user marshaling errors for COM-interop or PInvoke, which may corrupt the stack.

Short sample code:

Dto

public class SimpleItem
{
    public Guid Id { get; }

    public List<int> Ints { get; }

    public SimpleItem(Guid id, List<int> ints)
    {
        Id = id;
        Ints = ints;
    }
}

Serialization

var context = new SerializationContext();
var serializer = context.GetSerializer<SimpleItem>();

var item = new SimpleItem(Guid.NewGuid(), new List<int>() { 5, 11 });
var serializedItem = serializer.PackSingleObject(item);
var deserializedItem = serializer.UnpackSingleObject<SimpleItem>(serializedItem );

Using this stupid code snippet, serializedItem is only 4 bytes long, which contains only the List and Guid prop is not there. Trying to unpack causes the exception above.

Now what I found is IF we modify the Guid Id {get;} property to have a public setter:

public Guid Id { get; set; }

Then everything works as expected and data is packed and unpacked correctly. And its weird cause setter makes PACKING not to break anymore...

I tried a few other combinations:

Works

public class SimpleItem
{
    public Guid Id { get; set }

    public List<int> Ints { get; set }
}

**Does NOT work***

public class SimpleItem
{
    public readonly Guid Id;
    public List<int> Ints { get; }

    public SimpleItem(Guid id, List<int> ints)
    {
        Id = id;
        Ints = ints;
    }
}

Works

public class SimpleItem
{
    public Guid Id { get; }

    public SimpleItem(Guid id)
    {
        Id = id;
    }
}
``

**Works**
```cs
public class SimpleItem
{
    public List<int> Ints { get; }

    public SimpleItem(List<int> ints)
    {
        Ints = ints;
    }
}

Works

public class SimpleItem
{
    public Guid Id { get; }

    public JustInts Ints { get; }

    public SimpleItem(Guid id, JustInts ints)
    {
        Id = id;
        Ints = ints;
    }
}

public class JustInts
{
    public List<int> Ints { get; }

    public JustInts(List<int> ints)
    {
        Ints = ints;
    }
}

**Does NOT work. Messes up completely and debugger shows Id field as List and Ints field is null***

public class SimpleItem
{
    public string Id { get; }
    public List<int> Ints { get; }

    public SimpleItem(string id, List<int> ints)
    {
        Id = id;
        Ints = ints;
    }
}

Same as above...

public class SimpleItem
{
    public long Id { get; }
    public List<int> Ints { get; }

    public SimpleItem(long id, List<int> ints)
    {
        Id = id;
        Ints = ints;
    }
}

So as it seems as long as Guid (or some other primitive type) and List are at the same level (class), and Guid property does not have a public setter, something wrong happens.

I have not tried other collection types instead of List.

yfakariya commented 7 years ago

Thank you for reporting! I will investigate this issue.

yfakariya commented 7 years ago

This is caused by constructor based deserialization handling.

yfakariya commented 7 years ago

Fixed in 2cf6562