[API proposal] Attribute for passing caller identity implicitly

[jkotas]

Rationale

Number of libraries, including the new minimal ASP.NET host, use Assembly.GetCallingAssembly() for convenience. Assembly.GetCallingAssembly() is slow and unreliable API, and also not compatible with AOT technologies (e.g. dotnet/runtime#53825). We need a fast, reliable and AOT-friendly replacement of Assembly.GetCallingAssembly().

Proposal

namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = false)]
    public sealed class CallerIdentityAttribute : Attribute
    {
        public CallerIdentityAttribute();
    }
}

This attribute can be applied to parameters of type System.Reflection.Assembly, System.Reflection.Type, System.Reflection.MethodBase. When the C# compiler sees parameter tagged with this attribute, it will provide default value for it, according to the caller context:

• System.Reflection.Assembly: typeof(containing_type).Assembly
• System.Reflection.Type: typeof(containing_type)
• System.Reflection.MethodBase: MethodBase.GetMethodFromHandle(ldtoken containing_method)

Design meetings

• https://github.com/dotnet/csharplang/blob/main/meetings/2021/LDM-2021-09-22.md#calleridentityattribute
• https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-01-24.md#attribute-for-passing-caller-identity-implicitly

[ghost]

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

Issue Details

---

Rationale

Number of libraries, including the new minimal ASP.NET host, use Assembly.GetCallingAssembly() for convenience. Assembly.GetCallingAssembly() is slow and unreliable API, and also not compatible with AOT technologies (e.g. dotnet/runtime#53825). We need a fast, reliable and AOT-friendly replacement of Assembly.GetCallingAssembly().

Proposal

namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = false)]
    public sealed class CallerIdentityAttribute : Attribute
    {
        public CallerIdentityAttribute();
    }
}

This attribute can be applied to parameters of type System.Reflection.Assembly, System.Reflection.Type, System.Reflection.MethodBase. When the C# compiler sees parameter tagged with this attribute, it will provide default value for it, according to the caller context:

• System.Reflection.Assembly: typeof(containing_type).Assembly
• System.Reflection.Type: typeof(containing_type)
• System.Reflection.MethodBase: MethodBase.GetMethodFromHandle(ldtoken containing_method)

Author:	jkotas
Assignees:	-
Labels:	area-System.Runtime
Milestone:	-

[jkotas]

@jaredpar Thoughts on this proposal?

cc @davidfowl

[davidfowl]

So in the case of ASP.NET Core, it would look like this:

public class WebApplication
{
    public static WebApplicationBuilder CreateBuilder(string[] args, [CallerIdentityAttribute]Assembly assembly = null);
}

Usage:

var builder = WebApplication.CreateBuilder(args);

var app = builder.Build();

app.MapGet("/", context => context.Response.WriteAsync("Hello World!"));

app.Run();

The compiler would do this (since we're in Main):

var builder = WebApplication.CreateBuilder(args, typeof(Program).Assembly);

var app = builder.Build();

app.MapGet("/", context => context.Response.WriteAsync("Hello World!"));

app.Run();

That's slick. I like it.

[jkotas]

We should consider marking Assembly.GetCallingAssembly as obsolete as part of this feature.

[stephentoub]

Should this be in csharplang?

[jkotas]

Agree. This should start in csharplang.

[HaloFour]

Also see: #87

If those attributes were collapsed into a single attribute that passed a CallerInfo-like struct and the compiler were to pass the runtime method handle in that struct you should be able to derive the assembly from the declaring type of the calling method.

[jkotas]

I expect that we will frequently need to add trimming annotations like DynamicallyAccessedMembersAttribute to this argument. CallerInfo-like struct would not work well with these trimming annotations.

[jaredpar]

Overall this looks pretty reasonable to me.

For the Type and MethodBase values do we want to emit the actual types / methods in which the call occurs or the declared types. Consider as a concrete example the following:

class Program {
    static void Main() {
      PrintMe();
      var x = () => PrintMe(); 
      x();
   }

    static void PrintMe([CallerIdentityAttribute]Type type = null) => Console.WriteLine(type.FullName);
}

Does this print

Program
Program

Or

Program
<generated type name>

[davidfowl]

Good question. I think it depends on the usage. Should this work?

class Program {
    static void Main() {
      PrintMe();
      var x = () => PrintMe(); 
      x();
   }

    static void PrintMe([CallerIdentityAttribute]Type type = null) 
    {
       var m = type.GetMethod(nameof(PrintMe), BindingFlags.Public | BindingFlags.Static); // Does this return null?
    }
}

[jaredpar]

var m = type.GetMethod(nameof(PrintMe), BindingFlags.Public | BindingFlags.Static); // Does this return null?

That is explicitly undefined 😄.

The compiler does not specify how lambdas are emitted, except in special cases, because we want to be free to change it between versions. Hence the relationship in terms of metadata between PrintMe and the method generated for x is not defined. If the code does work in one version it doesn't imply it will work in the next version.

I know that often the language / framework says an item in metadata is undefined but in reality it never changes hence developers feel safe in taking dependencies on it. Lambda metadata is one area we've changed several times though and broken developers who took dependencies on it.

[jkotas]

do we want to emit the actual types / methods in which the call occurs or the declared types

I think it makes more sense to emit the declared types/methods, given that it is not specified how lambdas and other similar constructs are emitted.

[jkotas]

To make example in #4984 (comment) work as written, how are we going to retrofit the existing method to use the new pattern?

public class WebApplication
{
    // Existing method
    public static WebApplicationBuilder CreateBuilder(string[] args);
    // New method
    public static WebApplicationBuilder CreateBuilder(string[] args, [CallerIdentityAttribute] Assembly assembly = null);
}

I assume that WebApplication.CreateBuilder(args) is going to bind to the existing method. What is the best way to switch the existing code to the overload that uses this feature?

[jaredpar]

Believe we had the same overload issue with CallerArgumentExpression but not sure exactly what conclusion we came to. @333fred should remember

[stephentoub]

Believe we had the same overload issue with CallerArgumentExpression but not sure exactly what conclusion we came to

Yeah, it'd be nice if you had:

Debug.Assert(condition);

that we could change the method:

public static void Assert([DoesNotReturnIf(false)] bool condition, string? message)

to instead be:

public static void Assert([DoesNotReturnIf(false)] bool condition, [CallerArgumentExpression("condition")] string? message = null)

such that the compiler would emit:

Debug.Assert(condition, "condition");

Unfortunately, we already have the void Assert(bool condition) overload, so the compiler will bind to the "wrong" one.

[uecasm]

Outside of caller parameters, and more in line with #6041 (which was closed as a duplicate of this, hence posting here, though it does seem not entirely related), I often find myself needing the assembly name of the current type -- for example, to compose RCL wwwroot content paths of the form $"/_content/{AssemblyName}/some/sub/path.ext". It irks me that this is only available through reflection and not a compile-time const value, unless I hard-code a value which may become incorrect and introduce bugs due to a later rename or move refactoring.

[am11]

With MethodInfo, the callee can still obtain the type and assembly of the caller. (i.e. injectedMI?.DeclaringType?.Assembly)

[rickbrew]

I have an additional scenario/request for this feature, but I'm not sure it's an easy or even reasonable one, but I'd like to at least describe the scenario so it's on the radar. I am not using NativeAOT nor trimming in this context. Just crossgen/R2R.

This is in the context of Paint.NET's plugin systems and APIs. I very occasionally have a need to establish the identity of the calling method -- but, I also want to make sure that the caller cannot lie, spoof, or just get it wrong, and it'd be nice to not add additional visible complexity to things.

My proposal is to add a SkipFrames property onto the CallerIdentityAttribute so I can get the caller-of-the-caller. I think it's sufficient if this is limited to 0 or 1, it doesn't need the capability to go back 10 frames or whatever (so really it could be bool SkipOneFrame instead of an int).

public sealed class ClassThatPluginsUse
{
    // Maybe this would be required to have [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public ClassThatPluginsUse() : this((object?)null) { }

    private ClassThatPluginsUse(object? ignored, [CallerIdentity(SkipFrames = 1)] Assembly? callingAssembly = null)
    {
        // ... do something that depends on who the caller is
    }
}

I want the plugin to do new ClassThatPluginsUse(), which will call the other constructor, and then the SkipFrames property on the attribute will give me the identity of what called the parameterless constructor (i.o.w., the plugin). This permits me to get the identity of the plugin in a way that the plugin cannot spoof or get wrong. Today I would have to use Assembly.GetCallingAssembly() and [MethodImpl(MethodImplOptions.NoInlining)], which feels hacky and fragile.

I'm not going to make the argument that this would be a security feature 😂 This about wanting to be able to change behavior based on the app version that the plugin is compiled against. Call it a "plugin version contract" if you will. I can then call into callingAssembly.GetReferencedAssemblies() and dig through that list to see what version of (e.g.) PaintDotNet.Effects.dll the plugin was compiled against. This will allow me to version my plugin API in ways that are currently very difficult or impossible to achieve otherwise. This isn't something I need to use often, but when I need it I really need it.

I currently have a need for this in an API where I forgot to check a constructor parameter for null. This parameter is passed to the base class' private-protected constructor which does permit null. The proper way to get null down to that base constructor is to use the derived class' parameterless constructor:

public sealed class TabContainerControlInfo
    : StatefulControlInfo<TabContainerState, TabContainerStateProperty>
{
    // The private-protected base class constructor accepts `null` and this is normal. This parameterless constructor is the right way to do that.
    public TabContainerControlInfo()
        : base((TabContainerStateProperty?)null)
    {
    }

    // But I forgot to do a null-check here on the stateProperty parameter
    public TabContainerControlInfo(TabContainerStateProperty stateProperty)
        : base(stateProperty) // should be e.g. `stateProperty ?? throw new ArgumentNullException(nameof(stateProperty))`
    {
    }
    ...

This has resulted in some confusion and a bunch of wasted debugging time by plugin authors who were then getting a NullReferenceException later when they were using this TabContainerControlInfo in another part of the plugin system. You'll note that the constructor parameter is not annotated with a ?, so why didn't the compiler flag this? Well, these plugin authors are not enabling #nullable because ... they don't care, they don't know about it, they don't want to, they don't like it, etc. That part of the gospel has not reached them yet, in other words, and I can't force it upon them.

But I can't add the null check here without breaking the 10+ plugins that are already using this API incorrectly. Those plugins work fine today because they weren't using the TabContainerControlInfo in the other plugin APIs that would need the TabContainerStateProperty to be non-null (I realize this is confusing). These plugins should just be using the parameterless constructor. They're doing it the wrong way because they were following a tutorial that was doing this wrong, and the tutorial was wrong because it was carved out of another tutorial that did need to use the other plugin APIs that needed TabContainerStateProperty to be non-null (and it was doing all of that correctly).

I want to add the null-check, but only if the caller (the plugin) was compiled against the new version of my app. It's very hard to achieve this otherwise; I prototyped a system where I would use a ThreadLocal and some other Rube-Goldberg apparatuses and it kinda works but is fragile and also doesn't work in certain transitive cases (e.g. a plugin directly instantiating another plugin -- which is possible).

An alternative way of versioning this API would be to [Obsolete] the constructors and require folks to use a static Create method, but that's clunky and inconsistent -- everything else in this area is using constructors, not factory methods.

In Paint.NET I try to maintain strict binary compatibility for plugins, and there are plugins that are almost 20 years old that still work fine today. This is the best experience for the end-user/customer: breaking a plugin to correct an API does not really affect the plugin author, it only makes things worse for the end-user/customer by breaking their work/creative-flow. Having additional tools for versioning the plugin APIs makes it easier for me to uphold all of this.

[333fred]

@rickbrew I'm not sure how that would work though. This attribute, like all other caller info attributes, would need to be filled in at the call site by the compiler. How would the compiler insert the info from 2 frames up? And moreover, as with all caller info attributes, the user can simply provide whatever value they want manually if they so desire, so it really wouldn't be spoof-free either. It very much feels like, to get the functionality you want, you need to actually do the stack walk with a StackTrace to derive the info that way.

[rickbrew]

@rickbrew I'm not sure how that would work though. This attribute, like all other caller info attributes, would need to be filled in at the call site by the compiler. How would the compiler insert the info from 2 frames up? And moreover, as with all caller info attributes, the user can simply provide whatever value they want manually if they so desire, so it really wouldn't be spoof-free either. It very much feels like, to get the functionality you want, you need to actually do the stack walk with a StackTrace to derive the info that way.

Right, I understand -- but note that the constructor taking the Assembly is private, and I did also note in the code above that the middle-man constructor might be required to have [MethodImpl(MethodImplOptions.AggressiveInlining)]. Otherwise I don't think it's possible for the JIT or crossgen to have the right information to work with.

[rickbrew]

Also, by describing my scenario it may result in someone realizing "oh that can actually be achieved in this other way that is feasible to add support for".

[333fred]

[MethodImpl(MethodImplOptions.AggressiveInlining)]

This doesn't seem to help though? What is the compiler actually going to insert at the call site of the method?

[rickbrew]

[MethodImpl(MethodImplOptions.AggressiveInlining)]

This doesn't seem to help though? What is the compiler actually going to insert at the call site of the method?

I don't know I'm not really a compiler person ¯\(ツ)/¯ I'm not really trying to propose an implementation strategy, and I did note that I didn't know if it was feasible. I thought the scenario could be an interesting one to chew on. It's not unheard of for an API to forget to validate parameters but in a way that doesn't cause problems because they're passed on to other APIs where those parameter values are okay.

[rickbrew]

I'm also describing this because it seems there's a push to deprecate things like Assembly.GetCallingAssembly(), and I would be surprised if StackTrace wasn't in the crosshairs too, which means having alternatives that are compatible with modern .NET ideology (e.g. NativeAOT, trimming, reflection-free) could be important.

[MichalStrehovsky]

I'm also describing this because it seems there's a push to deprecate things like Assembly.GetCallingAssembly(), and I would be surprised if StackTrace wasn't in the crosshairs too, which means having alternatives that are compatible with modern .NET ideology (e.g. NativeAOT, trimming, reflection-free) could be important.

The problem with GetCallingAssembly is that it will do a stackwalk at runtime (same way as StackTrace) and give you the assembly of the calling previous frame. It may not be the assembly that called the current method (e.g. if the method got tail called, or the caller got inlined into something else). The API is basically just a trap that will look like it works most of the time, but then it doesn't. The "then it doesn't" part is going to be aggravated by tiered JIT and non-deterministic optimizations and those bugs are not fun bugs. This API can really only be used if you don't actually care about the result.

The API proposed here would be guaranteed to give the caller (unless the caller goes out of the way to spoof this but then ¯(ツ)/¯). The SkipFrames extension would just reintroduce the same problem because it would have to do a stack walk at runtime.

There are no plans to deprecate StackTrace. It can still be used wrong (like someone could use it to interpret it as a call chain and fall into the same trap as GetCallingAssembly), but this is a diagnostic API and returns what it says it returns - the current stack trace.

[JonathanBout]

It's been a year now. Is any progress made on this?

[CyrusNajmabadi]

It's been a year now. Is any progress made on this?

No. When we make progress we update the issues and discussions on the topic.

[FreeAndNil]

We need this feature for resolving apache/logging-log4net#233