Merge serialization changes to 2.0.0 #12020
Conversation
Removes SerializableAttribute from CoreCLR types not intended to be serializable as well as adding special handling to MulticastDelegate to prevent serializing delegates (which can't be correctly serialized cross platform/runtime).
Changes to throw PlatformNotSupportedException from ISerializable.GetObjectData and serialization constructors on non-Serializable types. Also removes private serialization constructors and some unneeded code that was used to support serializing non-serializable types. A few tests testing GetObjectData implementations are also removed. For Exception types, we call base instead of throwing from GetObjectData to be consistent with the many Exceptions that don't override GetObjectData
Throws PlatformNotSupportedException from IDeserializationCallback and IObjectReference implementations on types that are no longer serializable.
…ased serialization (dotnet#11910) * Variables renamed for reflection based serialization * Make EqualityComparers serialize like desktop * add missing interfaces * TimeZone serializable added * Internal hashtable serializable * Removed TimeZone as serializable type * Remove Lazy<T>'s [Serializable] attribute for 2.0 For performance, Lazy was completely rewritten for .NET Core 2.0 and has an entirely different format than desktop; trying to get it to match the desktop serialization format would require either reverting or providing a complicated custom serialization/deserialization implementation to try to match. Lazy can also wrap an Exception that occurred from trying to instantiate the object, and the only exception types that are serializable as of now in core are the base Exception and AggregateException. As such, we're cutting it from the list of supported types in 2.0. An easy workaround is simply to do what the implementation does: serialize lazy.Value instead of lazy. * tiny fixes to equalitycomparer.cs
Internal hashtable serialization attribute removed
|
We might still need #12010 CC @atsushikan |
|
@dotnet-bot test Windows_NT x64 CoreCLR Perf Tests Correctness please |
|
@morganbr let's merge this without Ati's change and include it with TimeZoneInfo changes |
|
I would like to merge what we are testing in CoreFX right now - which does not include Ati's change. |
|
@morganbr seems to be already in (everything before ~5/15 should be in) |
|
Obviously, please don't merge this without shiproom approval. (And testing your CoreFX PR with it of course) |
|
@danmosemsft FYI it's green, please merge once shiproom approves |
|
@danmosemsft this is working locally with CoreFX pending changes |
|
We should now also include the TimeZoneInfo fix: f32ce49 |
|
@ViktorHofer if that has been validated with corefx test please go ahead and update the 2.0 port PRs worth the fix and the text fix. |
ghost
mentioned this pull request
|
If you revert the changes to Assembly.cs, Module.cs and ParameterInfo.cs (i.e. exclude them from the "changeset"), you actually have the part of #12010 that should be in 2.0. I think makes a little more sense than a second Shiproom request to undo something you're changing as part of this Shiproom request. |
|
@atsushikan one would think, but it's essential I get this in today, and arguably that would reset testing. We can follow up right away with your change. |
…nCallback.OnDeserialization method. (dotnet#12024)
doesn't look related to our changes |
|
@krwq since we didn't change the runtime, and that test 47975 does not appear to exercise our code, I think this is safe to ignore. |
|
@dotnet-bot test Tizen armel Cross Release Build (running again since we have some time) |
danmoseley
added
the
* NO MERGE *
ViktorHofer
removed
the
* NO MERGE *
ghost
mentioned this pull request
Fix https://github.com/dotnet/corefx/issues/20574 The decision to make the runtime's implementation of Reflection objects non-serializable went too far and changed the behaviors of methods inherited by all Reflection implementors. In particular, by changing ParameterInfo.GetRealObject() to throw PNSE, all third-party implementations of ParameterInfo become undeserializable, not just the runtime's implementation. The intended change was to make the runtime's implementation non-serializable which is already accomplished by removing [Serializable] from RuntimeParameterInfo. The methods on the abstract base classes for Reflection are "apis" to all Reflection implementors and their long-standing behaviors should not change based on decisions made by the runtime's particular implementation. This commit rolls back those changes that were introduced as part of #12020.
Pending validation:
running through CI to see failures early.
cc: @danmosemsft @ViktorHofer @morganbr