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

Binary Serializing an object with a property of type 'Type' fails with SerializationException #23169

Closed
FransBouma opened this issue Aug 14, 2017 · 32 comments
Labels
area-System.Runtime question Answer questions and provide assistance, not an issue with source code or documentation.
Milestone

Comments

@FransBouma
Copy link
Contributor

(vs2017 15.3 preview 7.1. dotnet --version: 2.0.1-servicing-006924)

This code:

using System;
using System.Data.SqlClient;
using System.Diagnostics;
using System.IO;
using System.Runtime.Serialization.Formatters.Binary;

namespace ConsoleTester
{
    class Program
    {
        static void Main(string[] args)
        {
            BinaryFormatter formatter = new BinaryFormatter();
            MemoryStream stream = new MemoryStream();
            var toSerialize = new Foo() {Bar = "Bar", T = typeof(string)};
            formatter.Serialize(stream, toSerialize);
            stream.Seek(0, SeekOrigin.Begin);
            Foo deserialized = (Foo)formatter.Deserialize(stream);
            stream.Close();

            Console.WriteLine("Deserialized: " + deserialized.Bar);
        }
    }


    [Serializable]
    public class Foo
    {
        public string Bar { get; set; }
        public Type T { get; set; }
    }
}

results in:

Unhandled Exception: System.Runtime.Serialization.SerializationException: Type 'System.RuntimeType' in Assembly 'System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e' is not marked as serializable.
   at System.Runtime.Serialization.Formatters.Binary.WriteObjectInfo.InitSerialize(Object obj, ISurrogateSelector surrogateSelector, StreamingContext context, SerObjectInfoInit serObjectInfoInit, IFormatterConverter converter, ObjectWriter objectWriter, SerializationBinder binder)
   at System.Runtime.Serialization.Formatters.Binary.ObjectWriter.Write(WriteObjectInfo objectInfo, NameInfo memberNameInfo, NameInfo typeNameInfo)
   at System.Runtime.Serialization.Formatters.Binary.ObjectWriter.Serialize(Object graph, BinaryFormatterWriter serWriter, Boolean fCheck)
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream serializationStream, Object graph, Boolean check)
   at ConsoleTester.Program.Main(String[] args) in C:\Myprojects\VS.NET Projects\LLBLGen Pro v5.3\UnitTests\LLBLGen Pro\LowLevelAPI\NUnitTests\SelfServicing\ConsoleTester\ConsoleTester\Program.cs:line 16
Press any key to continue . . .

Looking at RuntimeType in CoreCli, it indeed isn't Serializable, while the type RuntimeType in .NET full is serializable.

This causes problems when porting code over from .NET full to .NET core 2.0 and using BinarySerialization. As the BinaryFormatter is available, one would assume it would work with normal arbitrary code like the simple class above.

@danmoseley
Copy link
Member

@ViktorHofer

@ViktorHofer
Copy link
Member

System.RuntimeType isn't serializable by design in .NET Core >=2.0. As I don't have any usage data for it I kindly ask @morganbr to give some intel on that.

A workaround for your scenario would be to change serializing the FullName of the Type, which is a string, instead of the type object itself. During deserialization you can create the full Type via Type.GetType(string).

@FransBouma
Copy link
Contributor Author

@ViktorHofer The error came as a bit of a surprise, when we ported our .NET full tests over to .NET core 2.0 and a test broke on this issue. It appears that we serialize the type a field has into the data so we can rebuild the graph as-is on the other side as otherwise it's not possible to obtain that data. I know the use-case isn't that common, but it is what it is ;)

The workaround you suggest is indeed the one we came up with as well, which works OK. I just posted it here as it could have been an oversight. I'm sure I'm not alone who will run into this, but I also realize BinarySerializer isn't a commonly used type.

@stephentoub
Copy link
Member

When we brought BinaryFormatter to .NET Core for 2.0, we also made serializable every type that was serializable in .NET Framework. But this also came with some heavy baggage: runtime serialization exposes lots of inner implementation details, which means optimizations we've already made and would like to make in the future can break roundtripping, differences between platforms can prevent roundtripping between different runtimes, etc. We had to choose between inhibiting the evolution of all of these types and supporting cross-version/cross-runtime serialization. We decided on supporting cross-version/cross-runtime serialization, but for only the most important types, based on an analysis of what types end up being used most commonly in code we have access to; we're hoping we hit a sweet spot that will both enable the majority use cases and that won't inhibit our ability to improve the platform. If problematic and impactful cases arise where it would be valuable to make additional types serializable in the future, we can do so, but it's much easier/safer to move in that direction that to try to take away serializability in core.

@FransBouma
Copy link
Contributor Author

@stephentoub good points. I understand making RuntimeType serializable could mean it breaks that cross version/cross runtime promise.

Anyway, to sum it up: there's a workaround, and there's a reason this is how it is today. People running into this issue later on can find this issue and see the workaround and reasoning behind it.

Thanks all :)

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 14, 2017

@FransBouma thanks for letting us know! We are encouraging our customers to try exactly these new features in 2.0 and feedback is extremely valuable.

We just finished our docs about binary serialization in .NET Core 2.0. Please find the list of supported types and some fair warnings about binary serialization here: https://docs.microsoft.com/en-us/dotnet/standard/serialization/binary-serialization

@FransBouma
Copy link
Contributor Author

FransBouma commented Aug 15, 2017

@ViktorHofer
Another one: System.Collections.CollectionBase. It's serializable on .net full, but not on .net core. I think more people will run into this one.

The main issue I think is that you simply get an error 'bla type isn't serializable' but when serializing a graph of objects, it's difficult to track down where exactly a certain type is used which causes this. Especially with a base class like CollectionBase it's used implicitly.

Again, it's not likely millions of users will have this particular problem, but porting exiting code might give this issue here for some people. In our particular case we have a collection class which is non-generic and which is based on CollectionBase (the code is at places 13+ years old, as is this part). To make it work we have to change the base type of this particular class to Collection<T>, which is a breaking change. We could opt for having a different base class based on platform (netstandard2.0 uses Collection<T>, netfx uses collectionbase) but that sucks too.

To give another example of why binary serialization is sometimes used: deep cloning of object graphs in-memory. We have users who want to deep clone in-memory graphs of entity objects and binary serialization a clean way to do that without fetching the graph again from the DB. (IMHO it's faster to fetch it again in many cases, but alas, customers do what customers do ;)) For this, binary serialization with everything the entity graph references should work so we run into these bits.

@FransBouma FransBouma reopened this Aug 15, 2017
@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 15, 2017

Thanks for getting back to us. Just for reference, these are the types in .NET Core which are inheriting from CollectionBase: http://source.dot.net/#System.Collections.NonGeneric/System/Collections/CollectionBase.cs,4e201fdc07f5f637,references.

I guess you are implementing your own custom collection which inherits from CollectionBase? As you stated, non-generic collections are old and already existed in the first versions of .NET. In many cases they were replaced by their generic counterparts in the BCL. We only have a few cases where we are using the non-generic versions. Using the generic collection types reduces allocation on the heap as of mitigation of boxing/unboxing. In this case I don't see an easy way besides refactoring the custom collection to use Collection<T> to support binary serialization. We definitely could add CollectionBase to the list of serializable types and ship it in a future servicing release but I believe that the current set of types present the most used ones. There is definitely some room for improvement (e.g. System.Lazy<T>) but I don't see it here in this case.

The main issue I think is that you simply get an error 'bla type isn't serializable' but when serializing a graph of objects, it's difficult to track down where exactly a certain type is used which causes this. Especially with a base class like CollectionBase it's used implicitly.

Correct. We could think about improving the error messages to emit the concrete type if the "not serializable one" is abstract.

To give another example of why binary serialization is sometimes used: deep cloning of object graphs in-memory.

Correct but would mean to again mark nearly every type as serializable which we don't want to do as it limits our means of future code changes. Possible solutions: own reflection based deep cloning (a lot of solutions around stackoverflow, copy constructor pattern (ICloneable), public API cloning with JSON.NET or other frameworks.

I understand your concerns and appreciate the feedback!

@FransBouma
Copy link
Contributor Author

I ran into another type, System.DBNull. This too isn't serializable it seems. This is problematic if you try to serialize a DataTable with null values (which are stored as DBNull.Value). People with typed datasets and using binary serialization will not be amused ;)

I think DBNull should be serializable because of that reason.

@danmoseley
Copy link
Member

One would think DBNull would be easy to support serializing, but I don't see it on the original lists of commonly serialized types that we based our list off. In desktop it is marked serializable but seems to throw? https://referencesource.microsoft.com/#mscorlib/system/dbnull.cs,25

@stephentoub
Copy link
Member

I don't see it on the original lists of commonly serialized types that we based our list off

The dangers of static analysis.

but seems to throw?

Its GetObjectData uses info.SetType to essentially tell the deserializer to use a different type to deserialize it:
https://referencesource.microsoft.com/#mscorlib/system/dbnull.cs,32
That's done so that it's deserialized as the DBNull singleton rather than as a unique instance.

@danmoseley
Copy link
Member

Ah yes. Do we have any infra in Core to support deserializing singletons? We don't have this UnitySerializationInfo.

@stephentoub
Copy link
Member

We had UnitySerializationInfo. It was deleted here:
dotnet/coreclr@b479cee#diff-11ac2b6cb4cabbcf76d5c4d20fbf66b5L24

@danmoseley
Copy link
Member

Thanks, I sh/could have found that myself. Given we support serializing SqlBoolean, etc -- is DBNull used where those guys are? If so it seems for consistency it could come along too.

@FransBouma
Copy link
Contributor Author

FransBouma commented Aug 18, 2017

@danmosemsft DBNull is a value returned by e.g. a datareader when a null value is returned by the database, but other than that I don't see the connection (no pun intended ;)). But I also don't see the use case of serializing a SqlBoolean, so perhaps in the use case where you found SqlBoolean should be serializable a DBNull is required to be serializable too. What I do know is that any datatable / typed datatable/set with data can (and in most cases will) contain one or more DBNull values and binary serializing these objects will fail in that case (so having a binary serializable DataTable/set is thus at the moment IMHO a bit moot)

@danmoseley
Copy link
Member

@FransBouma do you know of any other types, not listed in https://docs.microsoft.com/en-us/dotnet/standard/serialization/binary-serialization, that are likely to be important to successfully serialize many DataTable/DataSets ?

@FransBouma
Copy link
Contributor Author

@danmosemsft not that I'm aware of. I ran into it in my tests when I ported our ORM to netcore2.0/netstandard2.0 and as we have the feature to support typed datatables, a couple of tests failed with the reason DBNull wasn't serializable, but other tests with typed datatables without NULL values worked fine. It's of course anecdotal evidence but to my knowledge if a datatable without NULL values serializes properly, the only reason it might not is that the user has created a column with a type that's not serializable (e.g. one which isn't currently on the list of supported serializable types). The rest of the types used in DataTable appeared to be serialize (and deserialize) properly :)

@danmoseley
Copy link
Member

OK I opened an issue and I'll see if I can put someone on it. Possibly we can get it into a servicing release.

@danmoseley
Copy link
Member

Unless you want to take it of course :)

@JonHanna
Copy link
Contributor

I don't see it on the original lists of commonly serialized types that we based our list off.

Was that list produced before DBNull was brought into corefx, perhaps. We were without it for quite a while.

@danmoseley
Copy link
Member

@JonHanna I believe @morganbr generated it by scanning desktop assets in Nuget.

@morganbr
Copy link
Contributor

The static analysis showed no serialization of DbNull.Value, but that's probably because you wouldn't create a field of type DbNull. DbNull.Value probably only shows up in collections or as Object.

@FransBouma
Copy link
Contributor Author

@danmosemsft If it's in corefx, sure, but I saw DBNull is in CoreCLR. I've never worked with that repo and as it's not straightforward I'll pass for now.

But perhaps I overlooked the type and it is in CoreFx? ( I couldn't find it, only in CoreCLR). I think the issue you created is in CoreFX but should be in CoreCLR? but perhaps that doesn't matter, not sure :)

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 19, 2017

I think the issue you created is in CoreFX but should be in CoreCLR? but perhaps that doesn't matter, not sure :)

We tend to create issues for mscorlib/System.Private.Corelib things here in corefx as in the end they will surface and customers will ask about it here, as you did :) But that's not a fixed rule but it makes sense as from time to time we move types around between corefx and coreclr. Another reason for it, we have our labels for the different technologies here like System.Runtime.Extensions, which is currently used for binary serialization. In CoreClr we only have one label for corelib.

But perhaps I overlooked the type and it is in CoreFx? ( I couldn't find it, only in CoreCLR).

You can search for types/members here in https://source.dot.net and even jump to the source via the link at the bottom right "Web Access": https://source.dot.net/#System.Private.CoreLib/shared/System/DBNull.cs,7faae4cef0a3f251 which brings me to the type in coreclr: https://github.com/dotnet/coreclr/blob/4704e9af61bd23695e382bc498553e457d3be77a/src/mscorlib/shared/System/DBNull.cs

It is defined in coreclr and exposed in System.Runtime in the ref assembly. How to check where it is exposed? You simply go to https://apisof.net (ignore the cert warning, we are working on that) and search for DbNull and choose the right one. Scroll down and find the entry .NET Core 2.0 - System.Runtime: https://apisof.net/catalog/System.DBNull. Searching for it then in corefx leads me to this place: https://github.com/dotnet/corefx/blob/master/src/System.Runtime/ref/System.Runtime.cs#L853

And ultimately a serialization test record for it should be defined in System.Runtime.Serialization.Formatters.Tests here similar to the others: https://github.com/dotnet/corefx/blob/master/src/System.Runtime.Serialization.Formatters/tests/BinaryFormatterTestData.cs#L252 and also added to some of the test DataTables here: https://github.com/dotnet/corefx/blob/master/src/System.Runtime.Serialization.Formatters/tests/BinaryFormatterTestData.cs#L202.

TL;DR

@joperezr
Copy link
Member

@FransBouma does it sound fair to close this issue for now, and re-open it in case you find one more type that is not in the list provided by @danmosemsft ?

@FransBouma
Copy link
Contributor Author

@joperezr Yes it's OK to close this. We're done porting our library to .netstandard and haven't found any more types but if we do in the future, I'll re-open it.

@jrgcubano
Copy link

@FransBouma some thoughts about how you resolve this? code example?

@FransBouma
Copy link
Contributor Author

@jrgcubano No real workarounds, so it's up to MS to fix this if there are types which need to be serializable. I really want to see System.Type to be serializable but until then, it's up to them.

@jrgcubano
Copy link

@FransBouma ok thanks! We are looking for some way that allows us to move forward without any breaking change in nhibernate/fluent-nhibernate#390

@morganbr
Copy link
Contributor

morganbr commented Apr 3, 2018

System.Type (or anything else reflection-related) is unlikely to become serializable for a couple of reasons:

  1. It doesn't translate well across runtimes, platforms, and versions. A given type may or may not exist, be in the same assembly, or have the same members.
  2. While .NET Core doesn't promise that binary serialization is more secure than it is with .NET Framework, reflection types are part of many serialization attack gadgets. Those gadgets should hopefully be harder to find on .NET Core without them.

@jrgcubano
Copy link

In case someone is in the same situation and finish looking at this issue. As a guide, I end up using a similar approach to Microsoft.Bot.Builder, using Surrogates to serialize System.Type and MemberInfo for BinaryFormatter.

@FransBouma
Copy link
Contributor Author

@jrgcubano that's a nice solution, thanks for sharing!

lauromoura referenced this issue in lauromoura/efl-mono-exml Jul 25, 2019
Made the TestLibrary part of the test assembly to avoid having to load
other dlls or compiling code on the fly.

Due to dotnet limitations in the issue linked below and as we are
loading the EFL# DLL directly, comment out the serialization code for
now.

https://github.com/dotnet/corefx/issues/23213
vitorsousasilva referenced this issue in expertisesolutions/efl-mono-exml Jul 25, 2019
Made the TestLibrary part of the test assembly to avoid having to load
other dlls or compiling code on the fly.

Due to dotnet limitations in the issue linked below and as we are
loading the EFL# DLL directly, comment out the serialization code for
now.

https://github.com/dotnet/corefx/issues/23213
DanielEggers referenced this issue in DanielEggers/ManagedEsent Sep 6, 2019
The type 'System.RuntimeType' in assembly 'System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e' is not marked as serializable by design.
Therefore the serialization of TKey und TValue in the PersistentDictionary fail if the ManagedEsent assembly is called from a .NET Standard 2.0 assembly which itself is called from a .NET Core application.
This is mentioned in issue microsoft#9.
A workaround is suggested by @ViktorHofer in issue 23213 (https://github.com/dotnet/corefx/issues/23213).
This PR implements that workaround.
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

9 participants