IObjectReference binary deserialization for objects with circular references crashes

[bahusoid]

Description

Consider the following object with circular references:

var myClass = new MyProxy
{
    Id = 1
};
myClass.Ref = new MyProxy
{
    Id = 2,
    Ref = myClass // circular reference myClass.Ref.Ref == myClass
};

MyProxy is on demand dynamically generated class so for serialization/deserialization IObjectReference interface is used to generate proxy if required.

myClass instance above can be successfully serialized but throws on deserialization:

System.Runtime.Serialization.SerializationException: The object with ID 3 implements the IObjectReference interface for which all dependencies cannot be resolved. The likely cause is two instances of IObjectReference that have a mutual dependency on each other.
   at System.Runtime.Serialization.ObjectManager.GetCompletionInfo(FixupHolder fixup, ObjectHolder& holder, Object& member, Boolean bThrowIfMissing)
   at System.Runtime.Serialization.ObjectManager.CompleteObject(ObjectHolder holder, Boolean bObjectFullyComplete)
   at System.Runtime.Serialization.ObjectManager.DoFixups()
   at System.Runtime.Serialization.Formatters.Binary.ObjectReader.Deserialize(BinaryParser serParser, Boolean fCheck)
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Deserialize(Stream serializationStream, Boolean check)
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Deserialize(Stream serializationStream)

Reproducible example: https://gist.github.com/bahusoid/0f1e1fd25d9d3e5f1a147b0a0fea44cf

I noticed that for some reasons during serialization myClass (Id = 1) is serialized twice. It seems the reason for deserialization exception. So looks like a problem in circular reference detection logic when IObjectReference involved.

Configuration

Reproduced on .NET 4.6 and .NET Core 3.1 Windows 10 x64

[GrabYourPitchforks]

Preliminary pre-triage thoughts: not a regression, suggest won't fix since it doesn't seem to meet the BF servicing bar.

[koszeggy]

@bahusoid: I have just found this issue and tested with my formatter just for curiosity (throws a very similar exception, which is actually expected here).

Circular references are actually handled quite well by BinaryFormatter. But if an IObjectReference is deserialized, then things get tricky because during the deserialization a proxy type is created (which is not even replaceable with the final instance so its usages must be tracked to resolve them later). Once the proxy is fully deserialized, the GetRealObject is called, and if there was any reference to it during the deserialization, then the possibly tracked usages now can be resolved with the final object. But if there is a reference to the final object that is being deserialized while restoring its proxy (so it could not be fully restored yet), then the task is not solvable.

Actually there is a way to make this work: In GetRealObject you can return a new instance of the represented object based on the already deserialized data, instead of returning a reference that could not be possibly completely deserialized yet.

See this UnitTest for better understanding, especially the SelfReferencerIndirect instances, whose UseValidWay determines whether deserialization can work or not. The first block of the test contains valid use cases whereas the last one throws SerializationExceptions. Debugging may help to understand the nuances that make the difference.

[bahusoid]

Actually there is a way to make this work: In GetRealObject you can return a new instance of the represented objec

@koszeggy The tricky part is how to restore circular reference after deserialization. At a quick glance it seems you are restoring self reference inside ctor (it's just a special case of circular reference) Maybe I'm wrong but I don't see how to make your solution work for multilevel hierarchy with circular reference at any level like obj.Ref.Ref.Ref.Ref == obj (there is no self references obj.Ref != obj)

I actually prototyped a workaround solution myself sometime ago: https://gist.github.com/bahusoid/ecdd9119c30fef5e08585761a015b96b

In short potential self reference properties should be serialized as non IObjectReference wrapper classes (wrapper class is extracted from actual IObjectReference implementation and used as a base for it. See MyProxyObjWrapper and MyProxyObjRef).

Update It doesn't work.. Circular references are not properly deserialized... :(

[koszeggy]

Yes, this is the solution I was talking about (aaand, this already works with my serializer, too)

Maybe I'm wrong but I don't see how to make your solution work for multilevel hierarchy with circular reference at any level

Here is the difference. Just like your solution, it adds an extra layer. Once it is deserialized, it can be set anywhere where the same reference was used in the original object graph, no matter how deep multilevel hierarchy was used.

Anyway, if your problem is solved, maybe you could just close this issue.

[bahusoid]

Here is the difference.

I still don't get how to restore circular references in your case. Can you modify my original example to make it work? Your example is about self references. There is no self references in my example (obj.Ref != obj) - so it's really different case and I don't see how to apply your solution.

if your problem is solved, maybe you could just close this issue.

Why? It's not solved - it's an ugly workaround. My example should work as is. Or at least API should be improved to provide an easy way to serialize/deserialize such objects.

[koszeggy]

My example should work as is.

But it's not possible to provide a general solution for such case. My serializer is a completely different implementation, and still ended up in the same behavior for this particular case. But feel free to fork my repo and debug your original snippet. You will see why it is not possible (or, if you can come up with something don't hesitate to create a pull request).

Btw, I don't think BinaryFormatter will be changed as it is being obsoleted and is planned to be removed soon.

[GrabYourPitchforks]

Closing as does not meet servicing bar per https://github.com/dotnet/designs/blob/main/accepted/2020/better-obsoletion/binaryformatter-obsoletion.md#all-outstanding-binaryformatter-related-issues-resolved-wont-fix-net-5.