Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XmlSerializer tries to use Reflection.Emit even if RuntimeFeature.IsDynamicCodeSupported is false #59167

Closed
rolfbjarne opened this issue Sep 15, 2021 · 8 comments · Fixed by #59386
Assignees
Milestone

Comments

@rolfbjarne
Copy link
Member

Description

We have a test case that fails with when IsDynamicCodeSupported=false (when executing in a FullAOT scenario on an iOS device):

[FAIL] Bug1820_GenericList : System.TypeLoadException : Could not load type 'Microsoft.Xml.Serialization.GeneratedAssembly.XmlSerializer1' from assembly 'Microsoft.GeneratedCode, Version=1.0.0.0' because the parent type is sealed.
    at System.Reflection.Emit.TypeBuilder.CreateTypeInfo()
    at System.Xml.Serialization.XmlSerializationILGen.GenerateBaseSerializer(String , String , String , CodeIdentifiers )
    at System.Xml.Serialization.TempAssembly.GenerateRefEmitAssembly(XmlMapping[] , Type[] , String )
    at System.Xml.Serialization.TempAssembly..ctor(XmlMapping[] , Type[] , String , String )
    at System.Xml.Serialization.XmlSerializer.GenerateTempAssembly(XmlMapping xmlMapping, Type type, String defaultNamespace, String location)
    at System.Xml.Serialization.XmlSerializer.GenerateTempAssembly(XmlMapping xmlMapping, Type type, String defaultNamespace)
    at System.Xml.Serialization.XmlSerializer..ctor(Type type, String defaultNamespace)
    at System.Xml.Serialization.XmlSerializer..ctor(Type type)
    at LinkSdk.Serialization.XmlSerializationTest.Response.get_Serializer()
    at LinkSdk.Serialization.XmlSerializationTest.Response.Deserialize(String xml)
    at LinkSdk.Serialization.XmlSerializationTest.Bug1820_GenericList()
    at System.Reflection.RuntimeMethodInfo.Invoke(Object , BindingFlags , Binder , Object[] , CultureInfo )

the actual test failure message is misleading: the problem is that RuntimeFeature.IsDynamicCodeSupported is false, so XmlSerializer shouldn't try to use Reflection.Emit in the first place.

Configuration

.NET RC 1

Regression?

With regards to legacy Xamarin.iOS: yes.

@MichalStrehovsky
Copy link
Member

@marek-safar Do we need an issue making sure we run libraries tests with the interpreter disabled? This feels like something that should have been caught in libs tests.

@marek-safar marek-safar added this to the 6.0.0 milestone Sep 16, 2021
@marek-safar marek-safar removed the untriaged New issue has not been triaged by the area owner label Sep 16, 2021
@marek-safar
Copy link
Contributor

@eerhardt could you help with getting this fixed for 6.0?

@MichalStrehovsky I agree that ideally this should be uncovered in libraries AOT mode run but right now we can run only the runtime tests in this mode. Running all libraries tests under AOT won't be trivial as there is a lot of tests and a lot of assumptions to fix.

/cc @SamMonoRT

@jkotas
Copy link
Member

jkotas commented Sep 17, 2021

We have a fix for this in the NativeAOT experiment that worked fine for us so far.

Look for the XmlSerializer.cs delta in dotnet/runtimelab@runtime-main...feature/NativeAOT

@eerhardt
Copy link
Member

Looking now.

@eerhardt eerhardt self-assigned this Sep 20, 2021
@eerhardt
Copy link
Member

Looks like I can port the changes of XmlSerializer from runtimelab / NativeAOT to runtime.

My biggest question is: how can I test this to ensure it works? @rolfbjarne - Do you have instructions on how to run your test that is failing? So I can ensure my change fixes it?

eerhardt added a commit to eerhardt/runtime that referenced this issue Sep 21, 2021
Add more checks to XmlSerializer to check the SerializationMode. Don't try to use Reflection.Emit if the SerializationMode is ReflectionOnly.

These changes were ported from
* dotnet/runtimelab#593
* dotnet/runtimelab#600

Fix dotnet#59167
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 21, 2021
@rolfbjarne
Copy link
Member Author

@eerhardt AFAIK the only way to be sure this works is to actually run in a FullAOT scenario (i.e. on an iOS device), but I don't know how to make that happen inside dotnet/runtime (I'm guessing there already are FullAOT tests somewhere in dotnet/runtime?)

@SamMonoRT
Copy link
Member

@imhameed - please can you verify if the newly enabled FullAOT lane has similar tests running ?

eerhardt added a commit that referenced this issue Sep 23, 2021
* XmlSerializer support for IsDynamicCodeSupported=false

Add more checks to XmlSerializer to check the SerializationMode. Don't try to use Reflection.Emit if the SerializationMode is ReflectionOnly.

These changes were ported from
* dotnet/runtimelab#593
* dotnet/runtimelab#600

Fix #59167

* Fix a bug in XmlSerializer.CanDeserialize when in ReflectionOnly mode.

* Port UAP code for CanDeserialize

* PR feedback

* Add a linker test to ensure linker option '--enable-opt sealer' works when IsDynamicCodeSupported==false.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 23, 2021
github-actions bot pushed a commit that referenced this issue Sep 23, 2021
Add more checks to XmlSerializer to check the SerializationMode. Don't try to use Reflection.Emit if the SerializationMode is ReflectionOnly.

These changes were ported from
* dotnet/runtimelab#593
* dotnet/runtimelab#600

Fix #59167
@eerhardt
Copy link
Member

Re-opening to backport to 6.0.

@eerhardt eerhardt reopened this Sep 23, 2021
Anipik pushed a commit that referenced this issue Sep 24, 2021
…lse (#59507)

* XmlSerializer support for IsDynamicCodeSupported=false

Add more checks to XmlSerializer to check the SerializationMode. Don't try to use Reflection.Emit if the SerializationMode is ReflectionOnly.

These changes were ported from
* dotnet/runtimelab#593
* dotnet/runtimelab#600

Fix #59167

* Fix a bug in XmlSerializer.CanDeserialize when in ReflectionOnly mode.

* Port UAP code for CanDeserialize

* PR feedback

* Add a linker test to ensure linker option '--enable-opt sealer' works when IsDynamicCodeSupported==false.

Co-authored-by: Eric Erhardt <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants