Skip to content

Add trimming annotations to DotNetObjectReference #41610

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

Merged
merged 5 commits into from
May 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,18 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<linker>
<assembly fullname="Microsoft.AspNetCore.Components.Web, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2026</argument>
<property name="Scope">member</property>
<property name="Target">M:Microsoft.AspNetCore.Components.Web.Infrastructure.JSComponentInterop.SetRootComponentParameters(System.Int32,System.Int32,System.Text.Json.JsonElement,System.Text.Json.JsonSerializerOptions)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2026</argument>
<property name="Scope">member</property>
<property name="Target">M:Microsoft.AspNetCore.Components.Web.WebEventData.ParseEventArgsJson(Microsoft.AspNetCore.Components.RenderTree.Renderer,System.Text.Json.JsonSerializerOptions,System.UInt64,System.String,System.Text.Json.JsonElement)</property>
</attribute>
<assembly fullname="Microsoft.AspNetCore.Components.Web, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2062</argument>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<linker>
<assembly fullname="Microsoft.AspNetCore.Components.WebAssembly.Authentication, Version=42.42.42.42, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
Expand All @@ -7,11 +7,5 @@
<property name="Scope">member</property>
<property name="Target">F:Microsoft.Extensions.DependencyInjection.WebAssemblyAuthenticationServiceCollectionExtensions.&lt;&gt;c__1`1.&lt;&gt;9__1_0</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2091</argument>
<property name="Scope">member</property>
<property name="Target">M:Microsoft.Extensions.DependencyInjection.WebAssemblyAuthenticationServiceCollectionExtensions.&lt;&gt;c__1`1.&lt;AddAuthenticationStateProvider&gt;b__1_0(System.IServiceProvider)</property>
</attribute>
</assembly>
</linker>
</linker>
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using static Microsoft.AspNetCore.Internal.LinkerFlags;

namespace Microsoft.JSInterop;

/// <summary>
Expand All @@ -13,7 +16,7 @@ public static class DotNetObjectReference
/// </summary>
/// <param name="value">The reference type to track.</param>
/// <returns>An instance of <see cref="DotNetObjectReference{TValue}" />.</returns>
public static DotNetObjectReference<TValue> Create<TValue>(TValue value) where TValue : class
public static DotNetObjectReference<TValue> Create<[DynamicallyAccessedMembers(JSInvokable)] TValue>(TValue value) where TValue : class
{
if (value is null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using Microsoft.JSInterop.Infrastructure;
using static Microsoft.AspNetCore.Internal.LinkerFlags;

namespace Microsoft.JSInterop;

Expand All @@ -13,7 +15,8 @@ namespace Microsoft.JSInterop;
/// To avoid leaking memory, the reference must later be disposed by JS code or by .NET code.
/// </summary>
/// <typeparam name="TValue">The type of the value to wrap.</typeparam>
public sealed class DotNetObjectReference<TValue> : IDotNetObjectReference, IDisposable where TValue : class
public sealed class DotNetObjectReference<[DynamicallyAccessedMembers(JSInvokable)] TValue> :
IDotNetObjectReference, IDisposable where TValue : class
{
private readonly TValue _value;
private long _objectId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
using System.Text;
using System.Text.Json;
using Microsoft.AspNetCore.Internal;
using static Microsoft.AspNetCore.Internal.LinkerFlags;

[assembly: MetadataUpdateHandler(typeof(Microsoft.JSInterop.Infrastructure.DotNetDispatcher))]
[assembly: MetadataUpdateHandler(typeof(Microsoft.JSInterop.Infrastructure.DotNetDispatcher.MetadataUpdateHandler))]

namespace Microsoft.JSInterop.Infrastructure;

/// <summary>
/// Provides methods that receive incoming calls from JS to .NET.
/// </summary>
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070", Justification = "Linker does not propogate annotations to generated state machine. https://github.com/mono/linker/issues/1403")]
public static class DotNetDispatcher
{
private const string DisposeDotNetObjectReferenceMethodName = "__Dispose";
Expand Down Expand Up @@ -394,7 +394,7 @@ private static (MethodInfo methodInfo, Type[] parameterTypes) GetCachedMethodInf
throw new ArgumentException($"The type '{type.Name}' does not contain a public invokable method with [{nameof(JSInvokableAttribute)}(\"{methodIdentifier}\")].");
}

static Dictionary<string, (MethodInfo, Type[])> ScanTypeForCallableMethods(Type type)
static Dictionary<string, (MethodInfo, Type[])> ScanTypeForCallableMethods([DynamicallyAccessedMembers(JSInvokable)] Type type)
{
var result = new Dictionary<string, (MethodInfo, Type[])>(StringComparer.Ordinal);

Expand Down Expand Up @@ -499,11 +499,16 @@ private static Assembly GetRequiredLoadedAssembly(AssemblyKey assemblyKey)
?? throw new ArgumentException($"There is no loaded assembly with the name '{assemblyKey.AssemblyName}'.");
}

private static void ClearCache(Type[]? _)
// don't point the MetadataUpdateHandlerAttribute at the DotNetDispatcher class, since the attribute has
// DynamicallyAccessedMemberTypes.All. This causes unnecessary trim warnings on the non-MetadataUpdateHandler methods.
internal static class MetadataUpdateHandler
{
_cachedMethodsByAssembly.Clear();
_cachedMethodsByType.Clear();
_cachedConvertToTaskByType.Clear();
public static void ClearCache(Type[]? _)
{
_cachedMethodsByAssembly.Clear();
_cachedMethodsByType.Clear();
_cachedConvertToTaskByType.Clear();
Comment on lines +508 to +510
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this change - but that's where I noticed this. These caches will effectively disable support for unloadable (collectible) assemblies - since they hold a string reference to the Type. Would be really nice if we didn't introduce more hurdles in this space in the new frameworks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitek-karas that's fair, but at the same time its a trade-off between perf and correctness on a very concrete scenario. Do we have a design/proposal on how to deal with these two concerns that we could apply across all frameworks?

In any case, that's something we should discuss outside of this PR, but if we come up with a reasonable design for it, I think we would incorporate it.

}
}

private readonly struct AssemblyKey : IEquatable<AssemblyKey>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using System.Text.Json;
using System.Text.Json.Serialization;
using static Microsoft.AspNetCore.Internal.LinkerFlags;

namespace Microsoft.JSInterop.Infrastructure;

internal sealed class DotNetObjectReferenceJsonConverter<TValue> : JsonConverter<DotNetObjectReference<TValue>> where TValue : class
internal sealed class DotNetObjectReferenceJsonConverter<[DynamicallyAccessedMembers(JSInvokable)] TValue> : JsonConverter<DotNetObjectReference<TValue>> where TValue : class
{
public DotNetObjectReferenceJsonConverter(JSRuntime jsRuntime)
{
Expand Down
2 changes: 1 addition & 1 deletion src/JSInterop/Microsoft.JSInterop/src/JSRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ internal long BeginTransmittingStream(DotNetStreamReference dotNetStreamReferenc
return streamId;
}

internal long TrackObjectReference<TValue>(DotNetObjectReference<TValue> dotNetObjectReference) where TValue : class
internal long TrackObjectReference<[DynamicallyAccessedMembers(JSInvokable)] TValue>(DotNetObjectReference<TValue> dotNetObjectReference) where TValue : class
{
if (dotNetObjectReference == null)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<linker>
<assembly fullname="Microsoft.JSInterop, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<assembly fullname="Microsoft.JSInterop, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2026</argument>
Expand Down Expand Up @@ -39,9 +39,9 @@
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2060</argument>
<argument>IL2055</argument>
<property name="Scope">member</property>
<property name="Target">M:Microsoft.JSInterop.Infrastructure.DotNetDispatcher.&lt;&gt;c.&lt;GetTaskByType&gt;b__14_0(System.Type,System.Reflection.MethodInfo)</property>
<property name="Target">M:Microsoft.JSInterop.Infrastructure.DotNetObjectReferenceJsonConverterFactory.CreateConverter(System.Type,System.Text.Json.JsonSerializerOptions)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
Expand All @@ -67,5 +67,11 @@
<property name="Scope">member</property>
<property name="Target">M:Microsoft.JSInterop.JSRuntimeExtensions.&lt;InvokeAsync&gt;d__4`1.MoveNext</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2111</argument>
<property name="Scope">member</property>
<property name="Target">M:Microsoft.JSInterop.Infrastructure.DotNetDispatcher.GetCachedMethodInfo(Microsoft.JSInterop.Infrastructure.IDotNetObjectReference,System.String)</property>
</attribute>
</assembly>
</linker>
</linker>
5 changes: 5 additions & 0 deletions src/Shared/LinkerFlags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,9 @@ internal static class LinkerFlags
/// Flags for a component
/// </summary>
public const DynamicallyAccessedMemberTypes Component = DynamicallyAccessedMemberTypes.All;

/// <summary>
/// Flags for a JSInvokable type.
/// </summary>
public const DynamicallyAccessedMemberTypes JSInvokable = DynamicallyAccessedMemberTypes.PublicMethods;
}