mbraceproject / FsPickler

A fast multi-format message serializer for .NET
http://mbraceproject.github.io/FsPickler/
MIT License
323 stars 54 forks source link

F# 4.1 struct records cannot be serialised #85

Closed varon closed 6 years ago

varon commented 7 years ago

Description

Trying to serialise a simple record type introduced in F# 4.1, which is marked as a struct fails.

Repro code

open MBrace.FsPickler
open System.IO

[<Struct>]
type Broken =
    { Value: int }

[<EntryPoint>]
let main _ = 
    let binary = FsPickler.CreateBinarySerializer()
    let broken = { Value = 1 }
    let ms = new MemoryStream()
    binary.Serialize(ms, broken)
    0

Expected behavior

Ideally it should work, or provide a clearer error.

Actual behavior

From this example, an exception is thrown:

MBrace.FsPickler.FsPicklerException occurred
  HResult=0x80131500
  Message=Error serializing object of type 'Program+Broken'.
  Source=FsPickler
  StackTrace:
   at MBrace.FsPickler.RootSerialization.writeRootObject[T](IPicklerResolver resolver, ReflectionCache reflectionCache, IPickleFormatWriter formatter, FSharpOption`1 streamingContext, FSharpOption`1 sifter, Boolean isHash, Boolean disableSubtypes, Pickler`1 pickler, T value)
   at MBrace.FsPickler.FsPicklerSerializer.Serialize[T](Stream stream, T value, FSharpOption`1 pickler, FSharpOption`1 streamingContext, FSharpOption`1 encoding, FSharpOption`1 leaveOpen)
   at Program.main(String[] _arg1) in Program.fs:line 14
Inner Exception 1:
NullReferenceException: Object reference not set to an instance of an object.

I observed a lot of VERY obscure errors while attempting to serialise fairly complex F# 4.1 types in a larger project. These were non-determinstic, and included:

System.Runtime.InteropServices.SEHException: 'External component has thrown an exception.'

and

Managed Debugging Assistant 'FatalExecutionEngineError' : 'The runtime has encountered a fatal error. The address of the error was at 0x246d6a66, on thread 0x8d78. 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.'

and

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.

Finally, under a mono server which actually helped point me in the right direction, I received the following error in the larger project:

MBrace.FsPickler.PicklerGenerationException : Error generating pickler for type 'Test.MyStructRecordType'.
---- System.InvalidProgramException : Invalid IL code in (wrapper dynamic-method) MBrace.FsPickler.Emit/DynamicMethod/Marker:recordSerializer (MBrace.FsPickler.Pickler[],MBrace.FsPickler.WriteState,Test.MyStructRecordType): IL_0013: call      0x00000005

Related information

Workarounds: I'm going to try writing struct records by hand for now, as we did prior to 4.1.

varon commented 7 years ago

Reporting that the workaround of writing structs by hand in a pre-F# 4.1 does allow working around the issue, but isn't an elegant solution at all.

eiriktsarpalis commented 7 years ago

@varon this library has not been updated to take struct records into account. I pushed commit https://github.com/mbraceproject/FsPickler/commit/8b419014c64af6984a564197e8495624edd67aa7 so that the library reports proper error messages on struct record/union serialization.

varon commented 7 years ago

Thanks for the super fast response, @eiriktsarpalis!

Is this still being actively maintained? Is there any timeframe on an update? What would be involved in getting this up and running?

dsyme commented 7 years ago

@varon A group of us co-maintain the mbrace repos, so we can help guide you if you would like to contribute a PR (it would be great to have you on board).

It looks like it's mostly a matter of adjusting the codegen to take structnes into account. That can be quite painful though - you have to take extra addresses quite often when accessing fields and so on

varon commented 7 years ago

@dsyme - Thanks for the incredibly welcoming gesture.

I can shuffle some time around to get a PR for this together. It seems like a great opportunity to get some advanced mentorship on what seems to be a fairly clean but challenging codebase. I'll be happy to give it a bash if you can judge it's doable in a reasonable amount of time (for someone unfamiliar with the library internals).

I can probably figure out what to do, but a rough outline of the process would be great.

dsyme commented 7 years ago

From glancing at the code

varon commented 7 years ago

@dsyme - Thanks, I'll take a look.

varon commented 7 years ago

I'll need to add a reference to System.ValueTuple for the struct tuples to work. We may also need to upgrade F# 4.1 for that.

How would you like this to be done? NuGet? F# version selector?

varon commented 7 years ago

I've attempted this in what seems to be a reasonable way: https://github.com/varon/FsPickler/commit/405a883ec1a413f7023bbbf86c568a671f4e6473

If it helps, you can view the PR in progress here: https://github.com/varon/FsPickler/commits/fsharp41-compat

dsyme commented 7 years ago

@varon Great job so far, looking forward to the PR :)

thinkbeforecoding commented 7 years ago

I've seen that several PR have been merged to solve this bug. But it's not published on nuget yet ? Is there still work on this issue ?

eiriktsarpalis commented 6 years ago

Fixed in FsPickler 4.0