DataContractSerializer is not trimming safe #45559

marek-safar · 2020-12-03T18:30:30Z

Following code works today with linker enabled for SDK and user code but breaks with netcore libraries

public class Test
{
	// [Test]
	public static string SerializeSearchRequestWithDictionary ()
	{
		var req = new SearchRequest () {
			Query = "query",

			Users = new List<string> () {
				"user_a", "user_b"
			},

			Filters = new List<string> () {
				"filter_a", "filter_b"
			},

			Parameters = new Dictionary<string, string> () {
				{ "param_key_b", "param_value_a" },
				{ "param_key_a", "param_value_b" },
			}
		};

		using (MemoryStream memoryStream = new MemoryStream ()) {
			var dataContractSerializer = new DataContractSerializer (typeof (SearchRequest));
			dataContractSerializer.WriteObject (memoryStream, req);
			string serializedDataContract = Encoding.UTF8.GetString (memoryStream.ToArray (), 0, (int) memoryStream.Length);
		}
	}
}

[DataContract]
public class SearchRequest
{
	[DataMember]
	public string Query { get; set; }
	[DataMember]
	public List<string> Users { get; set; }
	[DataMember]
	public List<string> Filters { get; set; }
	[DataMember]
	public Dictionary<string, string> Parameters { get; set; }
}

Exception is thrown at runtime

System.Runtime.Serialization.InvalidDataContractException: NoGetMethodForProperty, LinkTestLib.SearchRequest, Query
   at System.Runtime.Serialization.DataContract.DataContractCriticalHelper.ThrowInvalidDataContractException(String message, Type type)
   at System.Runtime.Serialization.DataContract.DataContractCriticalHelper.ThrowInvalidDataContractException(String message)
   at System.Runtime.Serialization.ClassDataContract.ClassDataContractCriticalHelper.ImportDataMembers()
   at System.Runtime.Serialization.ClassDataContract.ClassDataContractCriticalHelper..ctor(Type type)
   at System.Runtime.Serialization.ClassDataContract..ctor(Type type)
   at System.Runtime.Serialization.DataContract.DataContractCriticalHelper.CreateDataContract(Type type)
   at System.Runtime.Serialization.DataContract.DataContractCriticalHelper.CreateDataContract(Int32 id, RuntimeTypeHandle typeHandle, Type type)
   at System.Runtime.Serialization.DataContract.DataContractCriticalHelper.GetDataContractSkipValidation(Int32 id, RuntimeTypeHandle typeHandle, Type type)
   at System.Runtime.Serialization.DataContract.GetDataContractSkipValidation(Int32 id, RuntimeTypeHandle typeHandle, Type type)
   at System.Runtime.Serialization.DataContract.GetDataContract(RuntimeTypeHandle typeHandle, Type type, SerializationMode mode)
   at System.Runtime.Serialization.DataContract.GetDataContract(RuntimeTypeHandle typeHandle, Type type)
   at System.Runtime.Serialization.DataContract.GetDataContract(Type type)
   at System.Runtime.Serialization.DataContractSerializer.get_RootContract()
   at System.Runtime.Serialization.DataContractSerializer.InternalWriteStartObject(XmlWriterDelegator writer, Object graph)
   at System.Runtime.Serialization.DataContractSerializer.InternalWriteObject(XmlWriterDelegator writer, Object graph, DataContractResolver dataContractResolver)
   at System.Runtime.Serialization.XmlObjectSerializer.WriteObjectHandleExceptions(XmlWriterDelegator writer, Object graph, DataContractResolver dataContractResolver)
   at System.Runtime.Serialization.XmlObjectSerializer.WriteObjectHandleExceptions(XmlWriterDelegator writer, Object graph)
   at System.Runtime.Serialization.XmlObjectSerializer.WriteObject(XmlDictionaryWriter writer, Object graph)
   at System.Runtime.Serialization.XmlObjectSerializer.WriteObject(Stream stream, Object graph)
   at Test.SerializeSearchRequestWithDictionary()

@eerhardt @vitek-karas

The text was updated successfully, but these errors were encountered:

eerhardt · 2020-12-03T18:42:17Z

works today with linker enabled for SDK and user code

Do we know how/why it works today? No code is referencing the SearchRequest.Query property getter, so how is the linker working today on it? Why isn't the property getting trimmed when not using the netcore libraries?

marek-safar · 2020-12-03T18:49:42Z

Do we know how/why it works today?

There are different custom linker steps which are trying to guess what should be kept based on some heuristic.

eerhardt · 2020-12-04T03:18:05Z

I don't believe this is going to be possible without more features from the ILLinker. We could make the exact scenario above work with the current [DynamicallyAccessedMembers] attribute. But as soon as SearchRequest has a property to another class, it will break again.

This issue is blocked by getting a serialization story in the ILLinker (dotnet/linker#1087), or implementing a source generator for DCS (#43545).

cc @MichalStrehovsky @mconnew

MichalStrehovsky · 2020-12-04T08:45:07Z

If I'm reading the code in MonoDroid.Tuner right, the heuristic basically relies on the DataContract/DataMember attributes to decide what to keep. Those attributes are not mandatory - you can drop them and the DCS will do the same thing for this class. But I assume MonoDroid.Tuner is going to break it the same as NetCore linker.

We'll need to decide how we want to treat "regression in a unit test" vs "regression in a scenario". This is a regression in a unit test, but I would assume the scenario (DCS) doesn't work either way.

marek-safar · 2020-12-04T08:59:22Z

This is a regression in a unit test, but I would assume the scenario (DCS) doesn't work either way.

Original customer issue is here https://bugzilla.xamarin.com/show_bug.cgi?id=36250

MichalStrehovsky · 2020-12-04T09:11:29Z

Original customer issue is here https://bugzilla.xamarin.com/show_bug.cgi?id=36250

Thanks!

System.MissingMethodException

Constructor on type 'System.Runtime.Serialization.CollectionDataContract+GenericDictionaryEnumerator`2[[System.String, mscorlib, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.String, mscorlib, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]' not found.

The customer actually hit a different issue and this kind of looks like one of those in the bucket of: "can't solve this without introducing DynamicallyAccessedMemberKinds.DataContractSerializer and specializing the DCS rules in the linker" - they hit this without aggressive treeshaking of user code. The unit test happens to pass with aggressive treeshaking of user code because of how the fix was made but it wasn't the original bug report.

…for netcore (#5354) This is or will be handled by BCL libraries and XA tooling should try not to interfere with more fine tuned handling done by BCL. Simplifies `[Preserve]` attribute implementation and removes old code to preserve serialization, which doesn't work well with new .NET5/6 runtime libraries. (the old serialization code: https://github.com/xamarin/xamarin-android/blob/master/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/ApplyPreserveAttribute.cs#L57-L82) The serialization is currently [broken](dotnet/runtime#45559) and will be fixed in runtime libs. Part of `CustomLinkDescriptionPreserve` test is thus disabled. Also the attribute removal is now handled in illink. The serialization fix leads to great reduction of assemblies size in simple XA test app by cca 1MB, apkdiff before/after: Summary: - 1,199,411 Assemblies -55.66% (of 2,154,984) - 1,206,333 Package size difference -13.37% (of 9,024,291) Co-authored-by: Radek Doulik <[email protected]>

jeffschwMSFT · 2022-08-27T14:21:07Z

@StephenMolloy if this is not serialization, do you have a recommendation?

ghost · 2022-08-30T14:27:15Z

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

Following code works today with linker enabled for SDK and user code but breaks with netcore libraries

public class Test
{
	// [Test]
	public static string SerializeSearchRequestWithDictionary ()
	{
		var req = new SearchRequest () {
			Query = "query",

			Users = new List<string> () {
				"user_a", "user_b"
			},

			Filters = new List<string> () {
				"filter_a", "filter_b"
			},

			Parameters = new Dictionary<string, string> () {
				{ "param_key_b", "param_value_a" },
				{ "param_key_a", "param_value_b" },
			}
		};

		using (MemoryStream memoryStream = new MemoryStream ()) {
			var dataContractSerializer = new DataContractSerializer (typeof (SearchRequest));
			dataContractSerializer.WriteObject (memoryStream, req);
			string serializedDataContract = Encoding.UTF8.GetString (memoryStream.ToArray (), 0, (int) memoryStream.Length);
		}
	}
}

[DataContract]
public class SearchRequest
{
	[DataMember]
	public string Query { get; set; }
	[DataMember]
	public List<string> Users { get; set; }
	[DataMember]
	public List<string> Filters { get; set; }
	[DataMember]
	public Dictionary<string, string> Parameters { get; set; }
}

Exception is thrown at runtime

System.Runtime.Serialization.InvalidDataContractException: NoGetMethodForProperty, LinkTestLib.SearchRequest, Query
   at System.Runtime.Serialization.DataContract.DataContractCriticalHelper.ThrowInvalidDataContractException(String message, Type type)
   at System.Runtime.Serialization.DataContract.DataContractCriticalHelper.ThrowInvalidDataContractException(String message)
   at System.Runtime.Serialization.ClassDataContract.ClassDataContractCriticalHelper.ImportDataMembers()
   at System.Runtime.Serialization.ClassDataContract.ClassDataContractCriticalHelper..ctor(Type type)
   at System.Runtime.Serialization.ClassDataContract..ctor(Type type)
   at System.Runtime.Serialization.DataContract.DataContractCriticalHelper.CreateDataContract(Type type)
   at System.Runtime.Serialization.DataContract.DataContractCriticalHelper.CreateDataContract(Int32 id, RuntimeTypeHandle typeHandle, Type type)
   at System.Runtime.Serialization.DataContract.DataContractCriticalHelper.GetDataContractSkipValidation(Int32 id, RuntimeTypeHandle typeHandle, Type type)
   at System.Runtime.Serialization.DataContract.GetDataContractSkipValidation(Int32 id, RuntimeTypeHandle typeHandle, Type type)
   at System.Runtime.Serialization.DataContract.GetDataContract(RuntimeTypeHandle typeHandle, Type type, SerializationMode mode)
   at System.Runtime.Serialization.DataContract.GetDataContract(RuntimeTypeHandle typeHandle, Type type)
   at System.Runtime.Serialization.DataContract.GetDataContract(Type type)
   at System.Runtime.Serialization.DataContractSerializer.get_RootContract()
   at System.Runtime.Serialization.DataContractSerializer.InternalWriteStartObject(XmlWriterDelegator writer, Object graph)
   at System.Runtime.Serialization.DataContractSerializer.InternalWriteObject(XmlWriterDelegator writer, Object graph, DataContractResolver dataContractResolver)
   at System.Runtime.Serialization.XmlObjectSerializer.WriteObjectHandleExceptions(XmlWriterDelegator writer, Object graph, DataContractResolver dataContractResolver)
   at System.Runtime.Serialization.XmlObjectSerializer.WriteObjectHandleExceptions(XmlWriterDelegator writer, Object graph)
   at System.Runtime.Serialization.XmlObjectSerializer.WriteObject(XmlDictionaryWriter writer, Object graph)
   at System.Runtime.Serialization.XmlObjectSerializer.WriteObject(Stream stream, Object graph)
   at Test.SerializeSearchRequestWithDictionary()

@eerhardt @vitek-karas

Author:	marek-safar
Assignees:	-
Labels:	`blocked`, `area-AssemblyLoader-coreclr`, `linkable-framework`
Milestone:	Future

eerhardt · 2022-08-30T14:35:48Z

Given dotnet/linker#1087 is closed as "not planned", and the plan on making Full trimming the default (dotnet/sdk#26246) I believe this issue should be Closed. The API above is marked as RequiresUnreferencedCode, which means the app developer will get a trim warning that the code won't work with trimming.

MichalStrehovsky · 2022-08-31T04:23:00Z

Given dotnet/linker#1087 is closed as "not planned", and the plan on making Full trimming the default (dotnet/sdk#26246) I believe this issue should be Closed. The API above is marked as RequiresUnreferencedCode, which means the app developer will get a trim warning that the code won't work with trimming.

This was actually fixed by grandfathering the MonoDroid behavior under a switch that Xamarin opts into. It doesn't make DCS trimming safe, but usable if you follow the very specific golden path that historically worked in Xamarin: https://github.com/dotnet/linker/blob/main/docs/serialization.md

dotnet/linker#1087 would not solve the general DCS serialization patterns either.

marek-safar added the linkable-framework Issues associated with delivering a linker friendly framework label Dec 3, 2020

Dotnet-GitSync-Bot added area-Serialization untriaged New issue has not been triaged by the area owner labels Dec 3, 2020

eerhardt added this to the 6.0.0 milestone Dec 3, 2020

eerhardt added the blocked Issue/PR is blocked on something - see comments label Dec 4, 2020

jonpryor mentioned this issue Dec 4, 2020

Remove custom types marking for types which XA does not own for netcore dotnet/android#5354

Merged

eerhardt modified the milestones: 6.0.0, Future May 11, 2021

jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Jul 8, 2021

StephenMolloy removed the area-Serialization label Aug 25, 2022

jeffschwMSFT added the area-AssemblyLoader-coreclr label Aug 30, 2022

jeffschwMSFT closed this as completed Aug 30, 2022

ghost locked as resolved and limited conversation to collaborators Sep 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DataContractSerializer is not trimming safe #45559

DataContractSerializer is not trimming safe #45559

marek-safar commented Dec 3, 2020

eerhardt commented Dec 3, 2020 •

edited

Loading

marek-safar commented Dec 3, 2020

eerhardt commented Dec 4, 2020

MichalStrehovsky commented Dec 4, 2020

marek-safar commented Dec 4, 2020

MichalStrehovsky commented Dec 4, 2020

jeffschwMSFT commented Aug 27, 2022

ghost commented Aug 30, 2022

eerhardt commented Aug 30, 2022

MichalStrehovsky commented Aug 31, 2022

DataContractSerializer is not trimming safe #45559

DataContractSerializer is not trimming safe #45559

Comments

marek-safar commented Dec 3, 2020

eerhardt commented Dec 3, 2020 • edited Loading

marek-safar commented Dec 3, 2020

eerhardt commented Dec 4, 2020

MichalStrehovsky commented Dec 4, 2020

marek-safar commented Dec 4, 2020

MichalStrehovsky commented Dec 4, 2020

jeffschwMSFT commented Aug 27, 2022

ghost commented Aug 30, 2022

eerhardt commented Aug 30, 2022

MichalStrehovsky commented Aug 31, 2022

eerhardt commented Dec 3, 2020 •

edited

Loading