Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Binary serialization is broken when building against netstandard and running on netfx. #300

Closed
safern opened this issue Apr 12, 2017 · 21 comments

Comments

@safern
Copy link
Member

safern commented Apr 12, 2017

If an assembly is built against netstandard and run it against netfx (desktop) and it has a type marked as Serialized and this types have any of the event methods that use either OnDeserializedAttribute, OnSerializingAttribute, OnSerializedAttribute, and OnDeserializingAttribute it would throw an exception like this one:

Type <type name> in assembly <assembly name> has method <method name> with an incorrect signature for the serialization attribute that it is decorated with

This would happen even if the method follows the guidelines stated in the msdn documentation:

To use the OnSerializingAttribute, the method must contain a StreamingContext parameter.

We are currently hitting this issue on the System.Runtime.Serialization.Formatters.Tests that have a type with this characteristics (source code) when the test is built against netstandard and we run it against netfx.

After investigating with @weshaggard he found out that there a bug in the methodtablebuilder.cpp when verifying the signature for this method (requiring a StreamingContext parameter) it doesn't follow the typeforwards correctly on the case it was defined on a shim like in the case of netstandard that is a typeforward to mscorlib so it throws an exception that the method signature is bad.

This bug is in Full Framework VM but we should decide if we want to document that we don't support applications/libraries using binary serialization built against netstandard and running on netfx.

See: https://github.com/dotnet/corefx/issues/17575#issuecomment-293657902

@jarenduan
Copy link

Could dropping support for these attributes in netstandard 2.0 be an alternative solution?

@weshaggard
Copy link
Member

@jarenduan yes that is an alternative approach. @terrajobst what do you think?

@danmoseley
Copy link
Member

@AlexGhiondea we might want to port this into full FX?

@danmoseley
Copy link
Member

Oh it's TFS 407239 already

@weshaggard
Copy link
Member

Yes we should try and get this fixed in .NET Framework.

@ericstj
Copy link
Member

ericstj commented Aug 29, 2017

Had a crazy idea for a workaround: have the netstandard lib reference an aliased MSCorlib from desktop targeting pack and then the user assembly can reference the StreamingContext from there. That'd get the right typeref in the user assembly that'd make desktop happy.

@weshaggard
Copy link
Member

I thought about that but that would cause all sorts of issues as netstandard is a core assembly and it cannot reference any other assemblies otherwise trouble will ensue.

@ericstj
Copy link
Member

ericstj commented Aug 29, 2017

I'm not talking about doing that in netstandard.dll. I'm talking about having the user assembly do it as a workaround until desktop could be fixed.

@weshaggard
Copy link
Member

Are you suggesting that the person building against .NET Standard also add a reference to the desktop mscorlib and then alias it just for these types? Wouldn't that cause some crazy amount of other ambiguous references? Or are you suggesting we produce a custom mscorlib with only StreamContext and whatever else is needed from the signature?

While that might work it seems very heavy handed and might cause issues on other platforms.

@ericstj
Copy link
Member

ericstj commented Aug 30, 2017

some crazy amount of other ambiguous references

No: because of the alias and the limited use of the serialization event methods (the runtime is the only one who typically calls them).

I'm not considering this as a serious long term solution, just a crazy idea for workaround if someone happens to be blocked. I agree its heavy handed.

@Levvente
Copy link

Levvente commented Jan 3, 2018

Hi!
Are there any updates regards this issue?
This was a really big hit for us, as we tried to port a huge codebase to netstandard2 and after extensive refactoring and working around not supported classes when we reached to testing all of our serialization codes are broken.
To say the least, I'm really disappointed regards the .Net Standard (Sorry, i'm not being rude, I'm just tired and stressful)

If you have any possible workarounds or any other information related to this bug please answer, I Hope there is some progress since August.

@weshaggard
Copy link
Member

Sorry to hear about your issues @Levvente. Unfortunately there isn't a lot we can do on the .NET Standard side of things to address this. It is a bug in the .NET Framework which takes a lot of time to update due to the risk of making changes to it. There isn't any great workarounds for the issue beyond not using the methods marked with the OnSerial*Attributes and instead do the serialization in other ways like the constructor (see https://github.com/dotnet/corefx/blob/a9bcc7965f6da027cd23d5d74c14b4ef6c61b313/src/System.Runtime.Serialization.Formatters/tests/SerializationTypes.cs#L282-L312 as an example).

@ViktorHofer
Copy link
Member

What Wes said is true, I just want to add that we are indeed working on a fix which will hopefully be ready for the next version of .NET Framework.

@ViktorHofer
Copy link
Member

TODO in corefx: When net472 is released, add its version to our version table in PlatformDetection and conditionalize the existing test to a) only run >=net472 or b) to fail with a TypeLoadException <net472.

@safern
Copy link
Member Author

safern commented Jan 11, 2018

Also would need to remove netfx from the tests configurations tu actually build ve netstandard and run in netfx.

https://github.com/dotnet/corefx/blob/master/src/System.Runtime.Serialization.Formatters/tests/Configurations.props

And remove the comment referencing the issue.

@rockfordlhotka
Copy link

Hello, I am wondering if there is any chance this will get resolved in the near future?

Until this is resolved many of us who build components and libraries for the community are stuck releasing a version for full .NET, and a version for netstandard 2.0, because the two just aren't compatible at runtime.

It is a PITA for us as community contributors, and an even bigger PITA for everyone out there who tries to use our NuGet packages!

@jherby2k
Copy link

I believe it’s fixed in 4.7.2 but I haven’t confirmed personally.

@ilkengin
Copy link

I had also got the error and it is now working like a charm after I moved to .NET Framework 4.7.2. Thanks a lot!

lazy pushed a commit to dotnet/infer that referenced this issue Oct 24, 2018
Due to .NETFramework issues, trying to use OnDeserializingAttribute leads to failures in Runtime.
Relevant issue: dotnet/standard#300
lazy pushed a commit to dotnet/infer that referenced this issue Oct 25, 2018
Due to .NETFramework issues, trying to use OnDeserializingAttribute leads to failures in Runtime.
Relevant issue: dotnet/standard#300
@terrajobst terrajobst added this to the .NET Standard 2.1 milestone Mar 13, 2019
@wtgodbe
Copy link
Member

wtgodbe commented May 30, 2019

@ViktorHofer @safern is there anything still to be done here?

@ViktorHofer
Copy link
Member

Nothing to be done one the corefx side...

@safern
Copy link
Member Author

safern commented May 30, 2019

I don't think so, this would require a netfx fix and there is already an actionable item there.

@wtgodbe wtgodbe closed this as completed May 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests