From 798ad84afe11ca9d96db026816761763c734a4d4 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Tue, 4 Jun 2024 17:51:07 -0500 Subject: [PATCH] Annotate SignalR client for native AOT Addresses the following problems: 0. Fix up some infrastructure around TrimmingAttributes to be cleaner. - Don't need to condition when to include them. Just always include them if you need it, and the TFM checks will be done for you. - Remove duplicate, standalone attribute files 1. HubConnection and ReflectionHelper's usage of finding IAsyncEnumerable interface 2. HubConnection's usage of MakeGenericMethod when using a streaming reader (IAsyncEnumerable or ChannelReader). - The only idea I have here is to follow the same approach as we do in DependencyInjection and elsewhere, which is to check for `IsDynamicCodeSupported == false` && `IsValueType` and throw an exception. This enables people to get exceptions during F5, and not only after publishing. --- eng/TrimmableProjects.props | 4 + ...NetCore.DataProtection.Abstractions.csproj | 3 +- ...Microsoft.AspNetCore.DataProtection.csproj | 3 +- ...spNetCore.DataProtection.Extensions.csproj | 3 +- ...t.Extensions.FileProviders.Embedded.csproj | 3 +- ...Extensions.Diagnostics.HealthChecks.csproj | 3 +- .../Microsoft.Extensions.Identity.Core.csproj | 3 +- ...icrosoft.Extensions.Identity.Stores.csproj | 3 +- ...AspNetCore.Connections.Abstractions.csproj | 2 +- .../DynamicallyAccessedMemberTypes.cs | 99 ------------------- .../DynamicallyAccessedMembersAttribute.cs | 50 ---------- src/Shared/TrimmingAttributes.cs | 42 ++++---- .../csharp/Client.Core/src/HubConnection.cs | 46 +++++++-- ...soft.AspNetCore.SignalR.Client.Core.csproj | 4 +- ...Microsoft.AspNetCore.SignalR.Client.csproj | 1 + ....AspNetCore.Http.Connections.Client.csproj | 1 + ...t.AspNetCore.SignalR.Protocols.Json.csproj | 3 + .../src/Protocol/JsonHubProtocol.cs | 20 ++-- src/SignalR/common/Shared/ReflectionHelper.cs | 32 +++--- 19 files changed, 110 insertions(+), 215 deletions(-) delete mode 100644 src/Shared/CodeAnalysis/DynamicallyAccessedMemberTypes.cs delete mode 100644 src/Shared/CodeAnalysis/DynamicallyAccessedMembersAttribute.cs diff --git a/eng/TrimmableProjects.props b/eng/TrimmableProjects.props index aa4b06dc9e6d..3b28def7582c 100644 --- a/eng/TrimmableProjects.props +++ b/eng/TrimmableProjects.props @@ -85,8 +85,12 @@ + + + + diff --git a/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj b/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj index 9c98f96d2db2..1fe6c9dd19ba 100644 --- a/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj +++ b/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj @@ -16,8 +16,7 @@ Microsoft.AspNetCore.DataProtection.IDataProtector - + diff --git a/src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj b/src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj index 8f674ca01208..ddf556d6668c 100644 --- a/src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj +++ b/src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj @@ -18,8 +18,7 @@ - + diff --git a/src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj b/src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj index 4e0f462d5b01..5b44128f30b5 100644 --- a/src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj +++ b/src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj @@ -12,8 +12,7 @@ - + diff --git a/src/FileProviders/Embedded/src/Microsoft.Extensions.FileProviders.Embedded.csproj b/src/FileProviders/Embedded/src/Microsoft.Extensions.FileProviders.Embedded.csproj index c08185cc6a01..590c4e980042 100644 --- a/src/FileProviders/Embedded/src/Microsoft.Extensions.FileProviders.Embedded.csproj +++ b/src/FileProviders/Embedded/src/Microsoft.Extensions.FileProviders.Embedded.csproj @@ -43,7 +43,6 @@ - + diff --git a/src/HealthChecks/HealthChecks/src/Microsoft.Extensions.Diagnostics.HealthChecks.csproj b/src/HealthChecks/HealthChecks/src/Microsoft.Extensions.Diagnostics.HealthChecks.csproj index 2037d145a6d0..e23ac6202aca 100644 --- a/src/HealthChecks/HealthChecks/src/Microsoft.Extensions.Diagnostics.HealthChecks.csproj +++ b/src/HealthChecks/HealthChecks/src/Microsoft.Extensions.Diagnostics.HealthChecks.csproj @@ -22,8 +22,7 @@ Microsoft.Extensions.Diagnostics.HealthChecks.IHealthChecksBuilder - + diff --git a/src/Identity/Extensions.Core/src/Microsoft.Extensions.Identity.Core.csproj b/src/Identity/Extensions.Core/src/Microsoft.Extensions.Identity.Core.csproj index 350751c48f94..1174c460f250 100644 --- a/src/Identity/Extensions.Core/src/Microsoft.Extensions.Identity.Core.csproj +++ b/src/Identity/Extensions.Core/src/Microsoft.Extensions.Identity.Core.csproj @@ -15,8 +15,7 @@ - + diff --git a/src/Identity/Extensions.Stores/src/Microsoft.Extensions.Identity.Stores.csproj b/src/Identity/Extensions.Stores/src/Microsoft.Extensions.Identity.Stores.csproj index c4551bb2cd9b..5c0bc03e774c 100644 --- a/src/Identity/Extensions.Stores/src/Microsoft.Extensions.Identity.Stores.csproj +++ b/src/Identity/Extensions.Stores/src/Microsoft.Extensions.Identity.Stores.csproj @@ -15,8 +15,7 @@ - + diff --git a/src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj b/src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj index 0c1987f47275..25784a904c42 100644 --- a/src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj +++ b/src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj @@ -19,9 +19,9 @@ - + diff --git a/src/Shared/CodeAnalysis/DynamicallyAccessedMemberTypes.cs b/src/Shared/CodeAnalysis/DynamicallyAccessedMemberTypes.cs deleted file mode 100644 index 1795f38b89fc..000000000000 --- a/src/Shared/CodeAnalysis/DynamicallyAccessedMemberTypes.cs +++ /dev/null @@ -1,99 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -#if !NET5_0_OR_GREATER -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Diagnostics.CodeAnalysis; - -/// -/// Specifies the types of members that are dynamically accessed. -/// -/// This enumeration has a attribute that allows a -/// bitwise combination of its member values. -/// -[Flags] -internal enum DynamicallyAccessedMemberTypes -{ - /// - /// Specifies no members. - /// - None = 0, - - /// - /// Specifies the default, parameterless public constructor. - /// - PublicParameterlessConstructor = 0x0001, - - /// - /// Specifies all public constructors. - /// - PublicConstructors = 0x0002 | PublicParameterlessConstructor, - - /// - /// Specifies all non-public constructors. - /// - NonPublicConstructors = 0x0004, - - /// - /// Specifies all public methods. - /// - PublicMethods = 0x0008, - - /// - /// Specifies all non-public methods. - /// - NonPublicMethods = 0x0010, - - /// - /// Specifies all public fields. - /// - PublicFields = 0x0020, - - /// - /// Specifies all non-public fields. - /// - NonPublicFields = 0x0040, - - /// - /// Specifies all public nested types. - /// - PublicNestedTypes = 0x0080, - - /// - /// Specifies all non-public nested types. - /// - NonPublicNestedTypes = 0x0100, - - /// - /// Specifies all public properties. - /// - PublicProperties = 0x0200, - - /// - /// Specifies all non-public properties. - /// - NonPublicProperties = 0x0400, - - /// - /// Specifies all public events. - /// - PublicEvents = 0x0800, - - /// - /// Specifies all non-public events. - /// - NonPublicEvents = 0x1000, - - /// - /// Specifies all interfaces implemented by the type. - /// - Interfaces = 0x2000, - - /// - /// Specifies all members. - /// - All = ~None -} -#endif diff --git a/src/Shared/CodeAnalysis/DynamicallyAccessedMembersAttribute.cs b/src/Shared/CodeAnalysis/DynamicallyAccessedMembersAttribute.cs deleted file mode 100644 index 486a314aac6e..000000000000 --- a/src/Shared/CodeAnalysis/DynamicallyAccessedMembersAttribute.cs +++ /dev/null @@ -1,50 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -#if !NET5_0_OR_GREATER -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Diagnostics.CodeAnalysis; - -/// -/// Indicates that certain members on a specified are accessed dynamically, -/// for example through . -/// -/// -/// This allows tools to understand which members are being accessed during the execution -/// of a program. -/// -/// This attribute is valid on members whose type is or . -/// -/// When this attribute is applied to a location of type , the assumption is -/// that the string represents a fully qualified type name. -/// -/// If the attribute is applied to a method it's treated as a special case and it implies -/// the attribute should be applied to the "this" parameter of the method. As such the attribute -/// should only be used on instance methods of types assignable to System.Type (or string, but no methods -/// will use it there). -/// -[AttributeUsage( - AttributeTargets.Field | AttributeTargets.ReturnValue | AttributeTargets.GenericParameter | - AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.Method, - Inherited = false)] -internal sealed class DynamicallyAccessedMembersAttribute : Attribute -{ - /// - /// Initializes a new instance of the class - /// with the specified member types. - /// - /// The types of members dynamically accessed. - public DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberTypes memberTypes) - { - MemberTypes = memberTypes; - } - - /// - /// Gets the which specifies the type - /// of members dynamically accessed. - /// - public DynamicallyAccessedMemberTypes MemberTypes { get; } -} -#endif diff --git a/src/Shared/TrimmingAttributes.cs b/src/Shared/TrimmingAttributes.cs index aeb701d47b74..75da0a2e15e4 100644 --- a/src/Shared/TrimmingAttributes.cs +++ b/src/Shared/TrimmingAttributes.cs @@ -5,71 +5,74 @@ namespace System.Diagnostics.CodeAnalysis; +#if !NET7_0_OR_GREATER /// -/// Indicates that the specified method requires dynamic access to code that is not referenced -/// statically, for example through . +/// Indicates that the specified method requires the ability to generate new code at runtime, +/// for example through . /// /// -/// This allows tools to understand which methods are unsafe to call when removing unreferenced -/// code from an application. +/// This allows tools to understand which methods are unsafe to call when compiling ahead of time. /// [AttributeUsage(AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Class, Inherited = false)] -internal sealed class RequiresUnreferencedCodeAttribute : Attribute +internal sealed class RequiresDynamicCodeAttribute : Attribute { /// - /// Initializes a new instance of the class + /// Initializes a new instance of the class /// with the specified message. /// /// - /// A message that contains information about the usage of unreferenced code. + /// A message that contains information about the usage of dynamic code. /// - public RequiresUnreferencedCodeAttribute(string message) + public RequiresDynamicCodeAttribute(string message) { Message = message; } /// - /// Gets a message that contains information about the usage of unreferenced code. + /// Gets a message that contains information about the usage of dynamic code. /// public string Message { get; } /// /// Gets or sets an optional URL that contains more information about the method, - /// why it requires unreferenced code, and what options a consumer has to deal with it. + /// why it requires dynamic code, and what options a consumer has to deal with it. /// public string? Url { get; set; } } +#endif +#if !NET5_0_OR_GREATER /// -/// Indicates that the specified method requires the ability to generate new code at runtime, -/// for example through . +/// Indicates that the specified method requires dynamic access to code that is not referenced +/// statically, for example through . /// /// -/// This allows tools to understand which methods are unsafe to call when compiling ahead of time. +/// This allows tools to understand which methods are unsafe to call when removing unreferenced +/// code from an application. /// [AttributeUsage(AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Class, Inherited = false)] -internal sealed class RequiresDynamicCodeAttribute : Attribute +internal sealed class RequiresUnreferencedCodeAttribute : Attribute { /// - /// Initializes a new instance of the class + /// Initializes a new instance of the class /// with the specified message. /// /// - /// A message that contains information about the usage of dynamic code. + /// A message that contains information about the usage of unreferenced code. /// - public RequiresDynamicCodeAttribute(string message) + public RequiresUnreferencedCodeAttribute(string message) { Message = message; } /// - /// Gets a message that contains information about the usage of dynamic code. + /// Gets a message that contains information about the usage of unreferenced code. /// public string Message { get; } /// /// Gets or sets an optional URL that contains more information about the method, - /// why it requires dynamic code, and what options a consumer has to deal with it. + /// why it requires unreferenced code, and what options a consumer has to deal with it. /// public string? Url { get; set; } } @@ -414,3 +417,4 @@ internal enum DynamicallyAccessedMemberTypes /// All = ~None } +#endif diff --git a/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs b/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs index 6825679330f2..e111b803d794 100644 --- a/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs +++ b/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs @@ -825,19 +825,27 @@ private void LaunchStreams(ConnectionState connectionState, Dictionary(ConnectionState connectionState, string streamId, ChannelReader reader, CancellationTokenSource tokenSource) { diff --git a/src/SignalR/clients/csharp/Client.Core/src/Microsoft.AspNetCore.SignalR.Client.Core.csproj b/src/SignalR/clients/csharp/Client.Core/src/Microsoft.AspNetCore.SignalR.Client.Core.csproj index f26bf49f0333..16c662419d25 100644 --- a/src/SignalR/clients/csharp/Client.Core/src/Microsoft.AspNetCore.SignalR.Client.Core.csproj +++ b/src/SignalR/clients/csharp/Client.Core/src/Microsoft.AspNetCore.SignalR.Client.Core.csproj @@ -5,6 +5,7 @@ $(DefaultNetCoreTargetFramework);$(DefaultNetFxTargetFramework);netstandard2.0;netstandard2.1 Microsoft.AspNetCore.SignalR.Client enable + true @@ -16,11 +17,10 @@ + - - diff --git a/src/SignalR/clients/csharp/Client/src/Microsoft.AspNetCore.SignalR.Client.csproj b/src/SignalR/clients/csharp/Client/src/Microsoft.AspNetCore.SignalR.Client.csproj index edeac1edd808..de0d7377e23b 100644 --- a/src/SignalR/clients/csharp/Client/src/Microsoft.AspNetCore.SignalR.Client.csproj +++ b/src/SignalR/clients/csharp/Client/src/Microsoft.AspNetCore.SignalR.Client.csproj @@ -4,6 +4,7 @@ Client for ASP.NET Core SignalR $(DefaultNetCoreTargetFramework);$(DefaultNetFxTargetFramework);netstandard2.0 enable + true diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Microsoft.AspNetCore.Http.Connections.Client.csproj b/src/SignalR/clients/csharp/Http.Connections.Client/src/Microsoft.AspNetCore.Http.Connections.Client.csproj index e79a1fd7bdba..2a04e266f950 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Microsoft.AspNetCore.Http.Connections.Client.csproj +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Microsoft.AspNetCore.Http.Connections.Client.csproj @@ -4,6 +4,7 @@ Client for ASP.NET Core Connection Handlers $(DefaultNetCoreTargetFramework);$(DefaultNetFxTargetFramework);netstandard2.0;netstandard2.1 enable + true diff --git a/src/SignalR/common/Protocols.Json/src/Microsoft.AspNetCore.SignalR.Protocols.Json.csproj b/src/SignalR/common/Protocols.Json/src/Microsoft.AspNetCore.SignalR.Protocols.Json.csproj index 0ee7265d3820..1070faf71fd8 100644 --- a/src/SignalR/common/Protocols.Json/src/Microsoft.AspNetCore.SignalR.Protocols.Json.csproj +++ b/src/SignalR/common/Protocols.Json/src/Microsoft.AspNetCore.SignalR.Protocols.Json.csproj @@ -8,9 +8,12 @@ Microsoft.AspNetCore.SignalR true enable + true + + diff --git a/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs b/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs index 16299dd504c3..35fdbf90f1a3 100644 --- a/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs +++ b/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs @@ -611,7 +611,7 @@ private void WriteCompletionMessage(CompletionMessage message, Utf8JsonWriter wr } else { - JsonSerializer.Serialize(writer, message.Result, _payloadSerializerOptions); + SerializeObject(writer, message.Result); } } } @@ -633,7 +633,7 @@ private void WriteStreamItemMessage(StreamItemMessage message, Utf8JsonWriter wr } else { - JsonSerializer.Serialize(writer, message.Item, _payloadSerializerOptions); + SerializeObject(writer, message.Item); } } @@ -691,7 +691,7 @@ private void WriteArguments(object?[] arguments, Utf8JsonWriter writer) } else { - JsonSerializer.Serialize(writer, argument, _payloadSerializerOptions); + SerializeObject(writer, argument); } } writer.WriteEndArray(); @@ -827,10 +827,7 @@ private static HubMessage BindInvocationMessage(string? invocationId, string tar return BindType(ref reader, type); } - private object? BindType(ref Utf8JsonReader reader, Type type) - { - return JsonSerializer.Deserialize(ref reader, type, _payloadSerializerOptions); - } + private object? BindType(ref Utf8JsonReader reader, Type type) => DeserializeObject(ref reader, type); private object?[] BindTypes(ref Utf8JsonReader reader, IReadOnlyList paramTypes) { @@ -914,6 +911,15 @@ private static HubMessage ApplyHeaders(HubMessage message, Dictionary JsonSerializer.Serialize(writer, value, _payloadSerializerOptions); + + [UnconditionalSuppressMessage("Trimming", "IL2026:RequiresUnreferencedCode", Justification = "See SerializeObject Justification above.")] + [UnconditionalSuppressMessage("AOT", "IL3050:RequiresDynamicCode", Justification = "See above.")] + private object? DeserializeObject(ref Utf8JsonReader reader, Type type) => JsonSerializer.Deserialize(ref reader, type, _payloadSerializerOptions); + internal static JsonSerializerOptions CreateDefaultSerializerSettings() { return new JsonSerializerOptions() diff --git a/src/SignalR/common/Shared/ReflectionHelper.cs b/src/SignalR/common/Shared/ReflectionHelper.cs index e310fa31b835..70351533572f 100644 --- a/src/SignalR/common/Shared/ReflectionHelper.cs +++ b/src/SignalR/common/Shared/ReflectionHelper.cs @@ -15,7 +15,7 @@ internal static class ReflectionHelper { // mustBeDirectType - Hub methods must use the base 'stream' type and not be a derived class that just implements the 'stream' type // and 'stream' types from the client are allowed to inherit from accepted 'stream' types - public static bool IsStreamingType([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Interfaces)] Type type, bool mustBeDirectType = false) + public static bool IsStreamingType(Type type, bool mustBeDirectType = false) { // TODO #2594 - add Streams here, to make sending files easy @@ -47,26 +47,28 @@ public static bool TryGetStreamType(Type streamType, [NotNullWhen(true)] out Typ return false; } - public static bool IsIAsyncEnumerable([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Interfaces)] Type type) + public static bool IsIAsyncEnumerable(Type type) => GetIAsyncEnumerableInterface(type) is not null; + + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", + Justification = "The 'IAsyncEnumerable<>' Type must exist and so trimmer kept it. In which case " + + "It also kept it on any type which implements it. The below call to GetInterfaces " + + "may return fewer results when trimmed but it will return 'IAsyncEnumerable<>' " + + "if the type implemented it, even after trimming.")] + public static Type? GetIAsyncEnumerableInterface(Type type) { - if (type.IsGenericType) + if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IAsyncEnumerable<>)) { - if (type.GetGenericTypeDefinition() == typeof(IAsyncEnumerable<>)) - { - return true; - } + return type; } - return type.GetInterfaces().Any(t => + foreach (Type typeToCheck in type.GetInterfaces()) { - if (t.IsGenericType) + if (typeToCheck.IsGenericType && typeToCheck.GetGenericTypeDefinition() == typeof(IAsyncEnumerable<>)) { - return t.GetGenericTypeDefinition() == typeof(IAsyncEnumerable<>); + return typeToCheck; } - else - { - return false; - } - }); + } + + return null; } }