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

JSON deserialization fails in iOS release build if class implements an interface #75802

Closed
markuspalme opened this issue Sep 17, 2022 · 48 comments
Assignees
Labels
area-VM-meta-mono linkable-framework Issues associated with delivering a linker friendly framework os-ios Apple iOS question Answer questions and provide assistance, not an issue with source code or documentation. trimming-for-aot `EnableAggressiveTrimming=true` used for running tests with AOT
Milestone

Comments

@markuspalme
Copy link

markuspalme commented Sep 17, 2022

Description

A simple JSON deserialization using Newtonsoft.Json fails on iOS in a release build as soon as the class that is trying be be de-serialized implements an interface.

The same app works in Debug build in the iOS simulator as expected.

Reproduction Steps

  1. Create a new iOS app with dotnet new ios

  2. Set a valid bundle identifier that allows installing the app on a physical device

  3. Add this code to the project:

    public class ProductImage : IEntity
    {
       public string ProductNumber { get; set; }
       public Guid Id { get; set; }
    }
      
    public interface IEntity
    {
       Guid Id { get; set; }
    }
    

    During startup, run this code:

    var x = @"{
       ""productNumber"": ""P1"",
       ""id"": ""46c67d7c-fd15-4d89-9fed-991c48c1bacf""
    }";
    
    var pi = JsonConvert.DeserializeObject<ProductImage>(x);
    
    
  4. Build the app for a device: dotnet publish testapp.csproj -f:net6.0-ios -c:Release /p:CodesignKey="Apple Development: ******" /p:CodesignProvision="Test app" /p:ArchiveOnBuild=true -r ios-arm64 --self-contained

  5. Deploy to device and run

Full project file:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net6.0-ios</TargetFramework>
    <OutputType>Exe</OutputType>
    <Nullable>enable</Nullable>
    <ImplicitUsings>true</ImplicitUsings>
    <SupportedOSPlatformVersion>13.0</SupportedOSPlatformVersion>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
  </ItemGroup>
</Project>

Self-contained example project:
https://github.com/markuspalme/dotnetruntime-75802

Expected behavior

The app starts successfully and manages to de-serialize the JSON regardless of whether the class implements an interface or not.

Note that removing the interface will make the de-serialization work as expected.

Actual behavior

JSON deserialization fails with this exception:

Error setting value to 'Id' on 'testapp.ProductImage'.

Regression?

The same code works in Xamarin.iOS.

Known Workarounds

No response

Configuration

.NET 6.0.401
iOS 16, same on 15.6.1

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 17, 2022
@ghost
Copy link

ghost commented Sep 17, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

A simple JSON deserialization using Newtonsoft.Json fails on iOS in a release build as soon as the class that is trying be be de-serialized implements an interface.

The same app works in Debug build in the iOS simulator as expected.

Reproduction Steps

  1. Create a new iOS app with dotnet new ios
  2. Set a valid bundle identifier that allows installing the app on a physical device
  3. Add this code to the project:

  public class ProductImage : IEntity
  {
	  public string ProductNumber { get; set; }
  
	  public Guid Id { get; set; }
  }
  
  public interface IEntity
  {
	  Guid Id { get; set; }
  }
 

During startup, run this code:

var x = @"{
      ""productNumber"": ""P1"",
      ""id"": ""46c67d7c-fd15-4d89-9fed-991c48c1bacf""
  }";

var pi = JsonConvert.DeserializeObject<ProductImage>(x);

  1. Build the app for a device: dotnet publish testapp.csproj -f:net6.0-ios -c:Release /p:CodesignKey="Apple Development: ******" /p:CodesignProvision="Test app" /p:ArchiveOnBuild=true -r ios-arm64 --self-contained

  2. Deploy to device and run

Full project file:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net6.0-ios</TargetFramework>
    <OutputType>Exe</OutputType>
    <Nullable>enable</Nullable>
    <ImplicitUsings>true</ImplicitUsings>
    <SupportedOSPlatformVersion>13.0</SupportedOSPlatformVersion>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
  </ItemGroup>
</Project>

Expected behavior

The app starts successfully and manages to de-serialize the JSON regardless of whether the class implements an interface or not.

Note that removing the interface

Actual behavior

JSON deserialization fails with this exception:

Error setting value to 'Id' on 'testapp.ProductImage'.

Regression?

The same code works in Xamarin.iOS.

Known Workarounds

No response

Configuration

.NET 6.0.4.1
iOS 16

Other information

No response

Author: markuspalme
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@gregsdennis
Copy link
Contributor

Are you certain this is a bug in .Net? Seems like it's a Newtonsoft issue. If so, the bug should be filed in that repo.

@markuspalme
Copy link
Author

@gregsdennis It works just fine in net6 on Windows, MacOS and also in the iOS simulator - all with the same Newtonsoft.Json package. So I think this is a runtime or maybe AOT issue (iOS requires AOT).

@gregsdennis
Copy link
Contributor

Understood, but it might be that the package just needs handle a special case in this OS. There is the possibility that the issue is the package.

@markuspalme
Copy link
Author

Unlikely, the same code works with Xamarin.iOS.

@Cheesebaron
Copy link

@markuspalme could it be the linker stripping out stuff?

Could you try add <PublishTrimmed>false</PublishTrimmed> in your Project Properties and see if that helps?

@markuspalme
Copy link
Author

markuspalme commented Sep 19, 2022

@Cheesebaron Thanks for the suggestion, on iOS that is not an option though:

image

@marek-safar marek-safar added the os-ios Apple iOS label Sep 19, 2022
@ghost
Copy link

ghost commented Sep 19, 2022

Tagging subscribers to 'os-ios': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

A simple JSON deserialization using Newtonsoft.Json fails on iOS in a release build as soon as the class that is trying be be de-serialized implements an interface.

The same app works in Debug build in the iOS simulator as expected.

Reproduction Steps

  1. Create a new iOS app with dotnet new ios
  2. Set a valid bundle identifier that allows installing the app on a physical device
  3. Add this code to the project:

  public class ProductImage : IEntity
  {
	  public string ProductNumber { get; set; }
  
	  public Guid Id { get; set; }
  }
  
  public interface IEntity
  {
	  Guid Id { get; set; }
  }
 

During startup, run this code:

var x = @"{
      ""productNumber"": ""P1"",
      ""id"": ""46c67d7c-fd15-4d89-9fed-991c48c1bacf""
  }";

var pi = JsonConvert.DeserializeObject<ProductImage>(x);

  1. Build the app for a device: dotnet publish testapp.csproj -f:net6.0-ios -c:Release /p:CodesignKey="Apple Development: ******" /p:CodesignProvision="Test app" /p:ArchiveOnBuild=true -r ios-arm64 --self-contained

  2. Deploy to device and run

Full project file:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net6.0-ios</TargetFramework>
    <OutputType>Exe</OutputType>
    <Nullable>enable</Nullable>
    <ImplicitUsings>true</ImplicitUsings>
    <SupportedOSPlatformVersion>13.0</SupportedOSPlatformVersion>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
  </ItemGroup>
</Project>

Expected behavior

The app starts successfully and manages to de-serialize the JSON regardless of whether the class implements an interface or not.

Note that removing the interface will make the de-serialization work as expected.

Actual behavior

JSON deserialization fails with this exception:

Error setting value to 'Id' on 'testapp.ProductImage'.

Regression?

The same code works in Xamarin.iOS.

Known Workarounds

No response

Configuration

.NET 6.0.401
iOS 16, same on 15.6.1

Other information

No response

Author: markuspalme
Assignees: -
Labels:

area-System.Text.Json, untriaged, os-ios

Milestone: -

@markuspalme
Copy link
Author

I have tried System.Text.Json instead of Newtonsoft.Json. It does not throw an exception, but it does not set any property values.

var pi = System.Text.Json.JsonSerializer.Deserialize<ProductImage>(x);

@markuspalme
Copy link
Author

Here's a small self-contained example:
https://github.com/markuspalme/dotnetruntime-75802

@markuspalme
Copy link
Author

Adding <UseInterpreter>true</UseInterpreter> as suggested in this issue helps - but I don't think this should be neccesary:
xamarin/xamarin-macios#15961

@Cheesebaron
Copy link

@markuspalme UseInterpreter is not for Release builds, Apple won't allow that.

@markuspalme
Copy link
Author

@Cheesebaron Yes, I figured but wanted to share that it helps here - maybe it helps pinpointing the root cause.

@ghost
Copy link

ghost commented Sep 19, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

A simple JSON deserialization using Newtonsoft.Json fails on iOS in a release build as soon as the class that is trying be be de-serialized implements an interface.

The same app works in Debug build in the iOS simulator as expected.

Reproduction Steps

  1. Create a new iOS app with dotnet new ios

  2. Set a valid bundle identifier that allows installing the app on a physical device

  3. Add this code to the project:

    public class ProductImage : IEntity
    {
       public string ProductNumber { get; set; }
       public Guid Id { get; set; }
    }
      
    public interface IEntity
    {
       Guid Id { get; set; }
    }
    

    During startup, run this code:

    var x = @"{
       ""productNumber"": ""P1"",
       ""id"": ""46c67d7c-fd15-4d89-9fed-991c48c1bacf""
    }";
    
    var pi = JsonConvert.DeserializeObject<ProductImage>(x);
    
    
  4. Build the app for a device: dotnet publish testapp.csproj -f:net6.0-ios -c:Release /p:CodesignKey="Apple Development: ******" /p:CodesignProvision="Test app" /p:ArchiveOnBuild=true -r ios-arm64 --self-contained

  5. Deploy to device and run

Full project file:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net6.0-ios</TargetFramework>
    <OutputType>Exe</OutputType>
    <Nullable>enable</Nullable>
    <ImplicitUsings>true</ImplicitUsings>
    <SupportedOSPlatformVersion>13.0</SupportedOSPlatformVersion>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
  </ItemGroup>
</Project>

Self-contained example project:
https://github.com/markuspalme/dotnetruntime-75802

Expected behavior

The app starts successfully and manages to de-serialize the JSON regardless of whether the class implements an interface or not.

Note that removing the interface will make the de-serialization work as expected.

Actual behavior

JSON deserialization fails with this exception:

Error setting value to 'Id' on 'testapp.ProductImage'.

Regression?

The same code works in Xamarin.iOS.

Known Workarounds

No response

Configuration

.NET 6.0.401
iOS 16, same on 15.6.1

Other information

No response

Author: markuspalme
Assignees: -
Labels:

area-System.Text.Json, untriaged, os-ios

Milestone: -

@eiriktsarpalis
Copy link
Member

@jeffschwMSFT I don't believe the issue is related to System.Text.Json. I removed the label but wasn't sure what the appropriate area label should be. Perhaps @steveisok or @akoeplinger know.

@jkotas
Copy link
Member

jkotas commented Sep 19, 2022

System.Text.Json should be the right area label.

Source generators are the only 100% reliable way to perform Json serialization/deserialization in the presence of trimming or AOT compilation without fallbacks.

Unfortunately, trim warnings that would notify about this problem are disabled for iOS apps by default. You can set <SuppressTrimAnalysisWarnings>false</SuppressTrimAnalysisWarnings> in your .csproj to enable trim warnings to see all potential places in your app that can break due to trimming.

This is duplicate #74141 and number of other similar issues.

@eiriktsarpalis My suggestion would be:

  • Make sure that we have documentation that we can point to and resolve these issues against. Do we have a documentation that says "System.Text.Json source generators are the only 100% reliable way to perform Json serialization/deserialization in the presence of trimming"?
  • Note the feedback on issues that track enabling the link warning in app-model specific SDKs. It is Don't suppress trim analysis warnings when linking all assemblies xamarin/xamarin-macios#11246 for iOS.

@eiriktsarpalis
Copy link
Member

System.Text.Json should be the right area label.

The OP concerns Json.NET (Newtonsoft) serialization issues in iOS. I agree that it falls under the same category as #74141, but I wonder what our approach should be when users report trimming-related failures when using third party reflection-based libraries. We might want to consider asking them to file an issue with said third-party libraries asking about AOT support.

Do we have a documentation that says "System.Text.Json source generators are the only 100% reliable way to perform Json serialization/deserialization in the presence of trimming"?

It is, although the wording is certainly more understated than that.

@markuspalme
Copy link
Author

markuspalme commented Sep 19, 2022

@jkotas I will try to enable the trim warnings.

JSON is at the heart of many apps and this feels like a regression coming from Xamarin.iOS where such scenarios worked out of the box. Migrating toSystem.Text.Json source generators is a possibility for user code, but many libraries depend on Newtonsoft.Json, consider a generic key-value store like Akavache (https://github.com/reactiveui/Akavache)

@eiriktsarpalis
Copy link
Member

@markuspalme I'm not sure if and when Json.NET plans on bringing AOT support, but you might want to consider opening an issue in its own repo or those of the other libraries that depend on it. In the meantime, migrating to System.Text.Json source generation is probably the only viable option.

@eiriktsarpalis eiriktsarpalis added question Answer questions and provide assistance, not an issue with source code or documentation. and removed untriaged New issue has not been triaged by the area owner labels Sep 19, 2022
@markuspalme
Copy link
Author

@eiriktsarpalis Thanks for your input. Given that earlier versions of Xamarin.iOS supported this scenario out of the box even in AOT mode, this seems like a bad surprise or even a blocker for anyone porting applications to net6-ios.

@markuspalme
Copy link
Author

@eiriktsarpalis So my understanding is that this problem here is caused by trimming. To preserve all the types in my code, I have added this to the project file as described here (https://devblogs.microsoft.com/dotnet/customizing-trimming-in-net-core-5/):

<ItemGroup>
  <TrimmerRootDescriptor Include="TrimmerRoots.xml" />
</ItemGroup>

The TrimmerRoots.xml looks like this:

<?xml version="1.0" encoding="utf-8"?>

<linker>
    <assembly fullname="testapp" preserve="all" />
</linker>

The de-serialization does not work with that in place. Shouldn't this ensure that all my types are preserved and available through reflection?

The whole story around trimming and linking is rather intimidating:

  • there is the [Preserve] attribute
  • there is the before mentioned external XML configuration option
  • For net6-ios projects there is the MtouchLink property in the project file

Is there any guidance available on which one to use when and how they work together?

@markuspalme
Copy link
Author

markuspalme commented Sep 20, 2022

@sbomer here you go:
build.binlog.zip

Build command:
dotnet publish testapp.csproj -f:net6.0-ios -c:Release /p:CodesignKey="Apple Development: ******" /p:CodesignProvision="Test app" /p:ArchiveOnBuild=true -r ios-arm64 --self-contained -bl:build.binlog

@markuspalme
Copy link
Author

@sbomer Did the binlog reveal anything interesting?

@steveisok
Copy link
Member

@sbomer if you need to bounce ideas off of someone who has ios set up, please let me know.

@sbomer
Copy link
Member

sbomer commented Oct 3, 2022

Sorry for the delay - I wasn't able to see anything wrong from the binlog, which indicates that the descriptor is getting passed along correctly. @vitek-karas would your repro tool help here to collect the input assemblies?

@sbomer
Copy link
Member

sbomer commented Oct 4, 2022

@markuspalme would you be able to try this tool by @vitek-karas to see if you can obtain a repro? https://github.com/vitek-karas/illinkrepro

You'll need to clone it on the machine where you ran the publish command and obtained the binlog. Then inside of the cloned directory, do dotnet run -- create path/to/msbuild.binlog. It tries creates a repro directory containing the linker command-line and input assemblies. If you can share that with us, it would be very helpful.

@markuspalme
Copy link
Author

markuspalme commented Oct 4, 2022

@sbomer Here you go: https://github.com/markuspalme/dotnetruntime-75802/raw/master/repro.zip

@sbomer
Copy link
Member

sbomer commented Oct 4, 2022

Thanks @markuspalme, that gets us a bit further. I was able to tweak the repro as I described here to get some linker output.

I was hoping that the linker output would show something obviously wrong, but I didn't notice anything. I see that ProductImage's interface implementation of IEntity is kept. It seems like it's actually kept with or without the descriptor, which is a bit surprising since the win-x64 repro I tried (without root descriptor) was failing with the interface and ctor being removed.

Would you also be able to share your publish output (ideally both with and without the descriptor) so I can compare? If that doesn't show what's wrong I will probably need to set up an iOS development environment to debug (or debug it with @steveisok).

@markuspalme
Copy link
Author

@sbomer Here are the IPA files:

With descriptor:
testapp-descriptor.ipa.zip

Without descriptor:
testapp-without-descriptor.ipa.zip

There is is at least a difference in size:

image

@sbomer
Copy link
Member

sbomer commented Oct 5, 2022

Thanks @markuspalme, in the output you shared I noticed that the ProductImage accessors don't look right:

Screenshot 2022-10-05 121113

I am not seeing the same thing in any of my repros so far. I also got your original repro to run on a simulator, but didn't see the issue there either. Now I'm trying to get set up with a provisioning profile with some help from @steveisok. Thanks for your patience.

@markuspalme
Copy link
Author

@sbomer I saw the same but was not sure it's relevant because I don't know if the assembly is used for anything but metadata in the AOT build.

The error is not reproducible on the simulator. I do think it's an AOT issue and not a trimming issue for this reason - but I have no idea how to investigate any further by myself.

@sbomer
Copy link
Member

sbomer commented Oct 5, 2022

So far I tend to agree that it doesn't look like a linker issue. I think we need someone more familiar with the AOT/Xamarin scenarios to investigate. /cc @steveisok

@MichalStrehovsky
Copy link
Member

Looks like the assembly has been ran through ILStrip after trimming. ILStrip builds on top of some unserviced version of Cecil. Is there a way to disable it to get it out of the picture? I can't figure out which targets enable it, but the methods are marked noininling and the bodies are just a ret and that's ILStrip.

I saw some hints in the .targets I found that ILStrip might be disabled if interpreter is enabled and we've established that enabling interpreted fixes it here #75802 (comment) so this might be ILStrip.

@MichalStrehovsky
Copy link
Member

Cc @akoeplinger for ILStrip

@akoeplinger
Copy link
Member

@MichalStrehovsky @markuspalme you can try setting EnableAssemblyILStripping to false in your csproj

@markuspalme
Copy link
Author

markuspalme commented Oct 6, 2022

@MichalStrehovsky I did that. The assembly is now intact:

image

But the error still occurs on device. As far as I know, the IL is used for metadata only with AOT enabled which is probably why method bodies are stripped away to save space.

@markuspalme
Copy link
Author

@MichalStrehovsky Is there anything else I can help with analyzing this issue?

@markuspalme
Copy link
Author

@MichalStrehovsky @steveisok @sbomer Could this be related to #69410?

@markuspalme
Copy link
Author

Can still be reproduced with .NET 7.

@minglu
Copy link

minglu commented Jan 31, 2023

following

@steveisok
Copy link
Member

@akoeplinger please investigate if this is indeed an ILStrip issue.

@steveisok
Copy link
Member

@akoeplinger do you the recent ILStrip changes might have fixed this? Can you please validate?

@markuspalme
Copy link
Author

This is fixed in 8.0.100-preview.7.23376.3, presumably by #69410.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-meta-mono linkable-framework Issues associated with delivering a linker friendly framework os-ios Apple iOS question Answer questions and provide assistance, not an issue with source code or documentation. trimming-for-aot `EnableAggressiveTrimming=true` used for running tests with AOT
Projects
None yet
Development

No branches or pull requests