Serialization of F#-defined exception variants doesn't serialize fields

[eiriktsarpalis]

This is an old issue with F# codegen which has left serialization in F#-defined exceptions using exception X = .... effectively broken. Here's a simple reproduction:

let clone (x : 'T) =
    let bf = new System.Runtime.Serialization.Formatters.Binary.BinaryFormatter()
    let m = new System.IO.MemoryStream()
    bf.Serialize(m, x)
    m.Position <- 0L
    bf.Deserialize(m) :?> 'T

exception Foo of x:string * y:int

let foo = Foo ("value", 42) 
clone foo // val it : exn = Foo (null,0)

The reason why this happens is that exceptions are necessarily serialized through ISerializable. Every type that inherits System.Exception must provide a SerializationInfo * StreamingContext constructor and an accompanying override of the ISerializable.GetObjectData() method, which are responsible for calling the corresponding base methods and serializing any additional data included in the subtype. In F# exceptions, generated code looks as follows (decompiled to C#):

    [CompilationMapping(SourceConstructFlags.Exception)]
    [Serializable]
    public class Foo : Exception, IStructuralEquatable
    {
        internal string x@;
        internal int y@;

        protected Foo(SerializationInfo info, StreamingContext context) : base(info, context)
        {
        }

        ...
    }

My proposal is to update codegen such that the generated class looks like so:

    [CompilationMapping(SourceConstructFlags.Exception)]
    [Serializable]
    public class Foo : Exception, IStructuralEquatable
    {
        internal string x@;
        internal int y@;

        protected Foo(SerializationInfo info, StreamingContext context) : base(info, context)
        {
            x@ = (string) info.GetValue("x", typeof(string));
            y@ = (int) info.GetValue("y", typeof(int));
        }

        public override GetObjectData(SerializationInfo info, StreamingContext context)
        {
            base.GetObjectData(info, context);
            info.AddValue("x", x@);
            info.AddValue("y", y@);
        }

        ...
    }

[KevinRansom]

@eiriktsarpalis Serializable and SerializationInfo are desktop clr artifacts. What would a portable and cross platform solution to this look like?

[dsyme]

@eiriktsarpalis I believe that is already implemented in IlxGen.fs, but has #if BE_SECURITY_TRANSPARENT. This was because overriding GetObjectData requires full trust from the CLR (e.g. when the generated code was used in partial trust scenarios). This unfortunately would have made FSharp.Core unusable in partial trust, and IlxGen.fs had no way of knowing if code was being generated for full trust or partial trust. So the fix was not applied, which I guess makes the resolution "Won't Fix".

Once upon a time (e.g. SIlverlight days) we cared a lot about partial trust. However the use of the CLR in partial trust mode is now rare, so it probably doesn't matter, and we could in theory enable the codegen for .NET Framework targets. (Perhaps it's still used in that way on SQL CLR, though that environment imposed yet another bunch of adhoc restrictions, e.g. "no static fields").

Kevin is right that Serializable and SerialziationInfo are gone in CoreCLR, so any fix would be irrelevant there. I also don't think there's any likelihood of us doing some other codegen to support serialization though (e.g. emitting DataContractAttribtue).

Perhaps we should start a separate discussion thread about what serialization models will be available on CoreCLR and whether they are sufficient for the homogeneous serializability needed by MBrace, FsPickler and/or Vagabond. I wonder what's happening to DataContract, DataContractSerializer and NetDataContractSerializer. I presume XmlSerializer and BinarySerializer will have gone.

BTW it's interesting to see how VM-level-isolation and VM-level-serialization are being dropped as design goals for CoreCLR.

[dsyme]

BTW the workaround to the bug is to define your exception as a class inheriting from System.Exception and define GetObjectData manually. That is, doing it the F# equivalent of the C# way.

[eiriktsarpalis]

Wow.

I'd be interested to find out what CoreCLR offers as an alternative for serializing exceptions. I was looking for the System.Exception implementation in the CoreCLR repo but could only find this. As you can see, it still contains the good old ISerializable methods, so this is certainly not the implementation used in the CoreCLR builds.

Serialization of exceptions is tricky, because of all the runtime metadata included (e.g. stack trace, inner exception, Data, Watson buckets, etc). It goes without saying that a correct exception serialization must carry all this metadata. This is why DataContract attributes cannot work in this context; as bad an abstraction as ISerializable may be, we are forced to use it in the context of exceptions. If the CoreCLR maintainers decided to simply cut ISerializable without providing an alternative serialization method for exception types, this would render them practically unserializable. Judging by this file, I suspect this to be the case. This is really bad news for distributed frameworks like mbrace or akka.net.

[dsyme]

@eiriktsarpalis Yeah, it looks like CoreCLR wants to get out of the business of defining serialization semantics and having built-in serialization logic (at least beyond the DataContract model). But equally you are right that it's critical for MBrace and Akka.NET to have good serialization of all sorts of base types including exceptions.

Do you think you could actually do serialization of exceptions as a special case in FsPickler, or is it just too hard?

CoreCLR seems to be taking the slice of .NET that's necessary for ASP.NET-style server-side workloads and console apps, and ditching things to do with isolation, .NET-to-.NET "homogeneous" communication and remoting. Equally, I can see why they are removing things much of this: .NET remoting in particular. But exceptions were probably originally made serializable to support .NET remoting....

It might be good to get some airtime with the .NET architects on this and get them to understand once more the value of up-stack distributed frameworks which use .NET-to-.NET "homogeneous" communication (i.e. which transport .NET objects), and to explain the problem.

[eiriktsarpalis]

Well this is certainly a case of throwing the baby out with the bathwater.

It wouldn't be too difficult to hardcode some kind of serialization logic in FsPickler, but this would certainly have to be done using a lot of reflection. It would practically mean that the serialization format is particular to that specific runtime implementation.

[eiriktsarpalis]

I just created an issue in the CoreCLR repo https://github.com/dotnet/coreclr/issues/2715

[enricosada]

@dsyme about you reply:

BTW it's interesting to see how VM-level-isolation and VM-level-serialization are being dropped as design goals for CoreCLR

I think everyone expect coreclr to compile to native, isolation by process.
see this comment on coreclr from coreclr team

• Security context: Use OS processes
• Assembly load context: Use https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Runtime/Loader/AssemblyLoadContext.cs

[dsyme]

@enricosada Thanks for that link. I added this comment: https://github.com/dotnet/coreclr/issues/295#issuecomment-172645262

I think everyone expect coreclr to compile to native, isolation by process.see this comment on coreclr from coreclr team

Perhaps :) Though when talking to .NET developers, a lot of them seem to assume that CoreCLR has the same feature set as .NET Framework.