Mapping gRPC services

[JamesNK]

I'm testing Grpc.AspNetCore.Server (.NET Core server implemention of gRPC) with member level trimming and I'm having trouble preserving a binding method that is used at startup and looked up using reflection.

In Startup.cs:

endpoints.MapGrpcService<ServiceImpl>();

User app:

public class ServiceImpl : ServiceBase
{
    public override void DoThing()
    {
        Console.WriteLine("Thing");
    }
}

Generated code (yeah it is kind of weird):

public static class ServiceStatic
{
    [BindServiceMethod(typeof(ServiceStatic), "BindService")]
    public class ServiceBase
    {
        public virtual void DoThing();
    }

    public static void BindService(Binder binder, ServiceBase serviceImpl)
    {
        binder.AddMethod(new Action(serviceImpl.DoThing));
    }
}

gRPC on the server is all generated code except for calling ServiceStatic.BindService at startup. That is looked up using reflection by finding the BindServiceMethod attribute.

I played around with linking attributes to include it, but I don't believe there is a combination that works. DynamicallyAccessedMembers can be used to preserve everything on ServiceImpl/ServiceBase, but the static binder method is on the parent static type:

public static GrpcServiceEndpointConventionBuilder MapGrpcService<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]TService>(this IEndpointRouteBuilder builder) where TService : class
{
    // ...
}

What could work in combination with MapGrpcService<[DynamicallyAccessedMembers]TService> is if DynamicDependency supported being placed on a type. If the type is included then its dynamic dependencies are also included.

Generated code could be updated to look like this:

public static class ServiceStatic
{
    [BindServiceMethod(typeof(ServiceStatic), "BindService")]
    [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(ServiceStatic))]
    public class ServiceBase
    {
        public virtual void DoThing();
    }

    public static void BindService(Binder binder, ServiceBase serviceImpl)
    {
        binder.AddMethod(new Action(serviceImpl.DoThing));
    }
}

MapGrpcService<ServiceImpl> references ServiceImpl which inherits from ServiceBase which then includes everything on ServiceStatic.

[MichalStrehovsky]

gRPC on the server is all generated code except for calling ServiceStatic.BindService at startup. That is looked up using reflection by finding the BindServiceMethod attribute.

Why cannot this part be done statically as well? E.g. there could be a C# 9 module initializer method that registers the BindService method (using a delegate) in a central location that e.g. maps from a Type to the binding delegate.

The purpose of the dataflow annotations is to make it possible for the linker to reason about reflection API usage. The fact that they end up keeping things is a bit of a side effect. Your proposed additions would only help to keep stuff, but linker would still have not know what type the GetMethod("BindService") operates on (I assume there is such line in the grpc codebase somewhere). That line would be tagged as linker unsafe. Having reflection code in the app that the linker cannot reason about is undesirable - trimming cannot be unit tested, it needs to be mechanically provable correct. We're trying to avoid introducing patterns that cannot be proved correct.

[MichalStrehovsky]

On a second thought - I see you have [BindServiceMethod(typeof(ServiceStatic), "BindService")]. Can you annotate the Type argument of this constructor as [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]?

I assume the custom attribute has a property/field that this value then flows to - this property/field can then also be annotated with the same annotations. With a bit of luck (if the processing is not too obscure), this annotation will then also make linker not warn about the GetMethod call (which is really what we want - we don't want to just keep stuff - we want linker to be able to prove that the trimming is safe).

[JamesNK]

I played around some more and placing DynamicDependency on a method does work:

public static class ServiceStatic
{
    [BindServiceMethod(typeof(ServiceStatic), "BindService")]
    public class ServiceBase
    {
        [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(ServiceStatic))]
        public virtual void DoThing();
    }

    public static void BindService(Binder binder, ServiceBase serviceImpl)
    {
        binder.AddMethod(new Action(serviceImpl.DoThing));
    }
}

While it works, it would get weird if there are multiple methods, e.g. DoThing and DoThing2. Is the attribute placed on all, or just the first method to satisfy the linker? etc

[JamesNK]

On a second thought - I see you have [BindServiceMethod(typeof(ServiceStatic), "BindService")]. Can you annotate the Type argument of this constructor as [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]?

I assume the custom attribute has a property/field that this value then flows to - this property/field can then also be annotated with the same annotations. With a bit of luck (if the processing is not too obscure), this annotation will then also make linker not warn about the GetMethod call (which is really what we want - we don't want to just keep stuff - we want linker to be able to prove that the trimming is safe).

BindServiceMethodAttribute comes from a third-party gRPC library but I should be able to get it updated with the attribute.

The attribute and reflection are used here - https://github.com/grpc/grpc-dotnet/blob/f418e5d5f770cc61e29b5c35ea30949623eb70a5/src/Shared/Server/BindMethodFinder.cs#L43-L66

internal static MethodInfo? GetBindMethodUsingAttribute(Type serviceType)
{
    Type? currentServiceType = serviceType;
    BindServiceMethodAttribute? bindServiceMethod;
    do
    {
        // Search through base types for bind service attribute
        // We need to know the base service type because it is used with GetMethod below
        bindServiceMethod = currentServiceType.GetCustomAttribute<BindServiceMethodAttribute>();
        if (bindServiceMethod != null)
        {
            // Bind method will be public and static
            // Two parameters: ServiceBinderBase and the service type
            return bindServiceMethod.BindType.GetMethod(
                bindServiceMethod.BindMethodName,
                BindMethodBindingFlags,
                binder: null,
                new[] { typeof(ServiceBinderBase), currentServiceType },
                Array.Empty<ParameterModifier>());
        }
    } while ((currentServiceType = currentServiceType.BaseType) != null);

    return null;
}

Would the linker be able to verify this?

[MichalStrehovsky]

Would the linker be able to verify this?

Yes, assuming the BindType property/field also gets annotated as [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] the linker should be able to understand that this is safe. It will keep all public methods and won't warn for the GetMethod call.

(Linker will also be able to see that BindMethodBindingFlags specifies to look at public methods only because it's a const.)

[JamesNK]

Ok, good to know!

As a library author, how do I enable linker warnings? I added <SuppressTrimAnalysisWarnings>false</SuppressTrimAnalysisWarnings> to the library csproj but I don't see any warnings when building.

I also tried including it as a flag when publishing the test app:

dotnet publish -r win10-x64 -c Release -p:PublishTrimmed=True -p:TrimMode=Link -p:SuppressTrimAnalysisWarnings=false

Instructions I followed: https://docs.microsoft.com/en-us/dotnet/core/deploying/trimming-options#analysis-warnings

[MichalStrehovsky]

Linker only runs during publish currently, so the property needs to be specified in the project that is getting published.

I think there is a way to run it on a library because we do it on the framework - we run it like that in the runtime repo (dotnet/runtime#40106 should be a breadcrumb that one could use to start hunting for that in .targets/.props). I don't know the details of that, but maybe @eerhardt @vitek-karas @sbomer would know.

[vitek-karas]

It's possible to run linker on a library alone, but it's not "nice" as it lack direct SDK support - it's in the plans to add support for this in the SDK. @sbomer will be able to provide details what to add and where to make it work.

[sbomer]

I don't have a copy-and-paste snippet to run it on a library, but if you are willing to put in some work you can do it by adding a call to the ILLink task during build. You'll need to pass reference assemblies to the task, and also options for roots, etc - see Microsoft.NET.ILLink.targets for how it's run during publish. We haven't tested it running against ref assemblies, so there may be bugs. I'd recommend running it during publish for now.

It sounds like you tried passing SuppressTrimAnalysisWarnings to publish, which should work. Are you still not seeing warnings? Passing properties on the command-line won't necessarily invalidate the incremental publish output, so one thing to try is cleaning obj and publishing again.

[eerhardt]

Just putting this here for informational purposes. This isn't how I'd expect real customers to do this, but this is what worked for me when I was annotating Microsoft.Extensions.* assemblies:

1. I made a linker-Extensions.response file with the following contents:

-reference "F:\git\runtime\artifacts\obj\System.Diagnostics.EventLog\netstandard2.0-Debug\System.Diagnostics.EventLog.dll"
-reference "F:\git\runtime\artifacts\obj\System.Security.Cryptography.Xml\netstandard2.0-Debug\System.Security.Cryptography.Xml.dll"
-reference "F:\git\runtime\artifacts\obj\System.Security.Permissions\net5.0-Debug\System.Security.Permissions.dll"
-reference "F:\git\runtime\artifacts\obj\System.Security.Cryptography.Pkcs\net5.0-Windows_NT-Debug\System.Security.Cryptography.Pkcs.dll"
-reference "F:\git\runtime\artifacts\obj\System.Windows.Extensions\net5.0-Windows_NT-Debug\System.Windows.Extensions.dll"
-reference "F:\git\runtime\artifacts\obj\System.Drawing.Common\net5.0-Windows_NT-Debug\System.Drawing.Common.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Win32.SystemEvents\net5.0-Windows_NT-Debug\Microsoft.Win32.SystemEvents.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Caching.Abstractions\netstandard2.0-Debug\Microsoft.Extensions.Caching.Abstractions.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Caching.Memory\netstandard2.0-Debug\Microsoft.Extensions.Caching.Memory.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Configuration\netstandard2.0-Debug\Microsoft.Extensions.Configuration.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Configuration.Abstractions\netstandard2.0-Debug\Microsoft.Extensions.Configuration.Abstractions.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Configuration.Binder\netstandard2.0-Debug\Microsoft.Extensions.Configuration.Binder.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Configuration.FileExtensions\netstandard2.0-Debug\Microsoft.Extensions.Configuration.FileExtensions.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Configuration.EnvironmentVariables\netstandard2.0-Debug\Microsoft.Extensions.Configuration.EnvironmentVariables.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Configuration.CommandLine\netstandard2.0-Debug\Microsoft.Extensions.Configuration.CommandLine.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Configuration.Ini\netstandard2.0-Debug\Microsoft.Extensions.Configuration.Ini.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Configuration.Json\netstandard2.1-Debug\Microsoft.Extensions.Configuration.Json.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Configuration.UserSecrets\netstandard2.0-Debug\Microsoft.Extensions.Configuration.UserSecrets.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Configuration.Xml\netstandard2.0-Debug\Microsoft.Extensions.Configuration.Xml.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.DependencyInjection\net5.0-Debug\Microsoft.Extensions.DependencyInjection.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.DependencyInjection.Abstractions\netstandard2.0-Debug\Microsoft.Extensions.DependencyInjection.Abstractions.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.DependencyModel\netstandard2.0-Debug\Microsoft.Extensions.DependencyModel.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.FileProviders.Abstractions\netstandard2.0-Debug\Microsoft.Extensions.FileProviders.Abstractions.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.FileProviders.Composite\netstandard2.0-Debug\Microsoft.Extensions.FileProviders.Composite.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.FileProviders.Physical\netstandard2.0-Debug\Microsoft.Extensions.FileProviders.Physical.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.FileSystemGlobbing\netstandard2.0-Debug\Microsoft.Extensions.FileSystemGlobbing.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Hosting\netstandard2.1-Debug\Microsoft.Extensions.Hosting.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Hosting.Abstractions\netstandard2.1-Debug\Microsoft.Extensions.Hosting.Abstractions.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Http\netstandard2.0-Debug\Microsoft.Extensions.Http.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Logging\netstandard2.1-Debug\Microsoft.Extensions.Logging.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Logging.Abstractions\netstandard2.0-Debug\Microsoft.Extensions.Logging.Abstractions.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Logging.Configuration\netstandard2.0-Debug\Microsoft.Extensions.Logging.Configuration.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Logging.Console\net5.0-Debug\Microsoft.Extensions.Logging.Console.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Logging.Debug\netstandard2.0-Debug\Microsoft.Extensions.Logging.Debug.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Logging.EventLog\netstandard2.0-Debug\Microsoft.Extensions.Logging.EventLog.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Logging.EventSource\net5.0-Debug\Microsoft.Extensions.Logging.EventSource.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Logging.TraceSource\netstandard2.0-Debug\Microsoft.Extensions.Logging.TraceSource.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Options\net5.0-Debug\Microsoft.Extensions.Options.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Options.ConfigurationExtensions\netstandard2.0-Debug\Microsoft.Extensions.Options.ConfigurationExtensions.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Options.DataAnnotations\netstandard2.1-Debug\Microsoft.Extensions.Options.DataAnnotations.dll"
-reference "F:\git\runtime\artifacts\obj\Microsoft.Extensions.Primitives\net5.0-Debug\Microsoft.Extensions.Primitives.dll"
-out "C:\temp\junk"
-r Microsoft.Extensions.Caching.Abstractions
-r Microsoft.Extensions.Caching.Memory
-r Microsoft.Extensions.Configuration
-r Microsoft.Extensions.Configuration.Abstractions
-r Microsoft.Extensions.Configuration.Binder
-r Microsoft.Extensions.Configuration.CommandLine
-r Microsoft.Extensions.Configuration.EnvironmentVariables
-r Microsoft.Extensions.Configuration.FileExtensions
-r Microsoft.Extensions.Configuration.Ini
-r Microsoft.Extensions.Configuration.Json
-r Microsoft.Extensions.Configuration.UserSecrets
-r Microsoft.Extensions.Configuration.Xml
-r Microsoft.Extensions.DependencyInjection
-r Microsoft.Extensions.DependencyInjection.Abstractions
-r Microsoft.Extensions.DependencyModel
-r Microsoft.Extensions.FileProviders.Abstractions
-r Microsoft.Extensions.FileProviders.Composite
-r Microsoft.Extensions.FileProviders.Physical
-r Microsoft.Extensions.FileSystemGlobbing
-r Microsoft.Extensions.Hosting
-r Microsoft.Extensions.Hosting.Abstractions
-r Microsoft.Extensions.Http
-r Microsoft.Extensions.Logging
-r Microsoft.Extensions.Logging.Abstractions
-r Microsoft.Extensions.Logging.Configuration
-r Microsoft.Extensions.Logging.Console
-r Microsoft.Extensions.Logging.Debug
-r Microsoft.Extensions.Logging.EventLog
-r Microsoft.Extensions.Logging.EventSource
-r Microsoft.Extensions.Logging.TraceSource
-r Microsoft.Extensions.Options
-r Microsoft.Extensions.Options.ConfigurationExtensions
-r Microsoft.Extensions.Options.DataAnnotations
-r Microsoft.Extensions.Primitives
-c skip -u skip 
-p link Microsoft.Extensions.Caching.Abstractions
-p link Microsoft.Extensions.Caching.Memory
-p link Microsoft.Extensions.Configuration
-p link Microsoft.Extensions.Configuration.Abstractions
-p link Microsoft.Extensions.Configuration.Binder
-p link Microsoft.Extensions.Configuration.CommandLine
-p link Microsoft.Extensions.Configuration.EnvironmentVariables
-p link Microsoft.Extensions.Configuration.FileExtensions
-p link Microsoft.Extensions.Configuration.Ini
-p link Microsoft.Extensions.Configuration.Json
-p link Microsoft.Extensions.Configuration.UserSecrets
-p link Microsoft.Extensions.Configuration.Xml
-p link Microsoft.Extensions.DependencyInjection
-p link Microsoft.Extensions.DependencyInjection.Abstractions
-p link Microsoft.Extensions.DependencyModel
-p link Microsoft.Extensions.FileProviders.Abstractions
-p link Microsoft.Extensions.FileProviders.Composite
-p link Microsoft.Extensions.FileProviders.Physical
-p link Microsoft.Extensions.FileSystemGlobbing
-p link Microsoft.Extensions.Hosting
-p link Microsoft.Extensions.Hosting.Abstractions
-p link Microsoft.Extensions.Http
-p link Microsoft.Extensions.Logging
-p link Microsoft.Extensions.Logging.Abstractions
-p link Microsoft.Extensions.Logging.Configuration
-p link Microsoft.Extensions.Logging.Console
-p link Microsoft.Extensions.Logging.Debug
-p link Microsoft.Extensions.Logging.EventLog
-p link Microsoft.Extensions.Logging.EventSource
-p link Microsoft.Extensions.Logging.TraceSource
-p link Microsoft.Extensions.Options
-p link Microsoft.Extensions.Options.ConfigurationExtensions
-p link Microsoft.Extensions.Options.DataAnnotations
-p link Microsoft.Extensions.Primitives
-t -b true --strip-substitutions false --strip-link-attributes false
--ignore-link-attributes true
--disable-opt unusedinterfaces 
--keep-dep-attributes true 
-d "F:\git\runtime\artifacts\bin\microsoft.netcore.app.runtime.win-x64\Debug\runtimes\win-x64\lib\net5.0"
-d "F:\git\runtime\artifacts\bin\microsoft.netcore.app.runtime.win-x64\Debug\runtimes\win-x64\native"

2. Invoke the linker with a command like dotnet "C:\Program Files\dotnet\sdk\5.0.100-rc.1.20452.10\Sdks\Microsoft.NET.ILLink.Tasks\tools\netcoreapp3.0\illink.dll" @<path-to-linker-Extensions.response>

This will invoke the linker and output all linker warnings for the assemblies you are developing.

If someone wants to try this out themselves, there are 4 parts in the above response file that need to be filled out:

1. -reference add the full paths to your built assemblies, and any other assemblies they reference outside of the netcoreapp package.
2. -r AssemblyName any assembly you want to be analyzed.
3. -p link AssemblyName any assembly you want to be analyzed.
4. -d <full path> to where the netcoreapp assemblies can be found. Typically this could be C:\Program Files\dotnet\shared\Microsoft.NETCore.App\5.0.0-rc.1.20451.14.

[JamesNK]

It sounds like you tried passing SuppressTrimAnalysisWarnings to publish, which should work. Are you still not seeing warnings? Passing properties on the command-line won't necessarily invalidate the incremental publish output, so one thing to try is cleaning obj and publishing again.

It started working after I commented so transient state in obj might have been the problem.

Another question: Do the linker types (DynamicallyAccessedMembers/DynamicallyAccessedMemberTypes) need to be referenced from .NET 5, or can internal copies with the same name and namespace be used? A library I want to annotate currently doesn't target .NET 5.

[vitek-karas]

Linker doesn't look for types in certain assemblies, it's similar to Roslyn in that it only looks at namespace/typename - so yes, you can declare them on your own and linker should be able to pick them up without any issues. If you do that you might also want to add annotations such that the attributes are removed after trimming the final app (as they're no longer necessary), but if this is worth it depends on the amount of usage.

[vitek-karas]

I don't see any remaining work here. The library trimming is described here: https://docs.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming