-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Using Queue, Queue<T>, Stack, and ConcurrentStack<T> as properties in classes deserialized by System.Text.Json does not work when trimmed #53205
Comments
Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer Issue DetailsPublish and run the following application with using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Text.Json;
namespace ConsoleApp1
{
class Program
{
static void Main(string[] args)
{
var json = @"{""Name"":""Test"",""Sizes"":[0, 256, 256]}";
var s = (Shape)JsonSerializer.Deserialize(json, typeof(Shape));
Console.WriteLine(s.Sizes.Dequeue());
Console.WriteLine(s.Sizes.Dequeue());
Console.WriteLine(s.Sizes.Dequeue());
}
}
public class Shape
{
public string Name { get; set; }
//public ConcurrentStack<int> Sizes { get; set; }
public Queue<int> Sizes { get; set; }
}
}
Expected resultsThe application should run, it does when I use Actual results
Notes
|
Should we use the same approach as with #53317 and not keep them all the time (e.g. ConcurrentStack sounds like rarely used for serialization) ? |
+1 We should have a trim-safe way to support these types - #53393. |
I did some investigation, and I don't think this is worth it. Looking through the collections that are hard-coded in JSON, I only find 4 that seem to be rarely used for serialization: In a default Blazor WASM app, the first three are all used elsewhere: public class ConcurrentStack<T> : IEnumerable<T>, IEnumerable, ICollection, IReadOnlyCollection<T>
{
public int Count
{
[MethodImpl(MethodImplOptions.NoInlining)]
get
{
throw new NotSupportedException("Linked away");
}
}
[MethodImpl(MethodImplOptions.NoInlining)]
public void Push(T P_0)
{
throw new NotSupportedException("Linked away");
}
[MethodImpl(MethodImplOptions.NoInlining)]
public IEnumerator<T> GetEnumerator()
{
throw new NotSupportedException("Linked away");
}
[MethodImpl(MethodImplOptions.NoInlining)]
IEnumerator IEnumerable.GetEnumerator()
{
throw new NotSupportedException("Linked away");
}
} Thus, doing the proposed work would only trim the For other applications that don't use ASP.NET or Microsoft.Extensions (and thus |
I think excluding Concurrent* from the implicit set of dependencies is still a good hygiene approach. |
@layomia has this been addressed in your recent collections support PR? |
Publish and run the following application with
-p:PublishTrimmed=true
dotnet publish -r win-x64 -p:PublishTrimmed=true
bin\Debug\net6.0\win-x64\publish\HelloWorld.exe
Expected results
The application should run, it does when I use
List<T>
for the property.Actual results
Notes
Queue
,Queue<T>
,Stack
, andConcurrentStack<T>
types.List<T>
norStack<T>
, because I think other things are rooting their constructors.System.Text.Json
serialization, and instead tell people to use the source generator if they need this behavior. The user already is getting a warning by using JsonSerializer without the source generated information.The text was updated successfully, but these errors were encountered: