khellang / Scrutor

Assembly scanning and decoration extensions for Microsoft.Extensions.DependencyInjection
MIT License
3.57k stars 233 forks source link

FromExecutingAssembly and FromCallingAssembly are misleading #92

Open justinbhopper opened 5 years ago

justinbhopper commented 5 years ago

FromExecutingAssembly and FromCallingAssembly are misleading. The naming suggests that it will use the assemblies from the perspective of where it is being called, but actually uses scrutor's perspective.

https://github.com/khellang/Scrutor/blob/master/src/Scrutor/TypeSourceSelector.cs#L20

        public IImplementationTypeSelector FromCallingAssembly()
        {
            return FromAssemblies(Assembly.GetCallingAssembly());
        }

        public IImplementationTypeSelector FromExecutingAssembly()
        {
            return FromAssemblies(Assembly.GetExecutingAssembly());
        }

Because these methods are defined in Scrutor, the resulting assemblies will be:

In other words,

khellang commented 5 years ago

Hmm. That's a very good point! 🤦‍♂️

The intention was to map directly to the Assembly methods, but I guess FromCallingAssembly doesn't make much sense as that would always be Scrutor. I guess I should just depreciate it and let the caller pass in their own assemblies.

I still think FromExecutingAssembly makes sense as that should be the actual executing assembly, i.e. the executable that is calling the code, no? 🤔

justinbhopper commented 5 years ago

@khellang I agree, FromExecutingAssembly makes sense and it can use Assembly.GetCallingAssembly internally. Although I would think it that would be a breaking change, I think it might be safe to assume no one is using FromExecutingAssembly assuming it is always resolving to Scrutor as of today.

And you can make FromCallingAssembly deprecated, because at best it can only do what it is currently doing today, which is resolving Scrutor's calling assembly (despite its name). I don't think Scrutor would be capable of resolving the calling code's calling assembly (Scrutor's grandparent, as it were), so there's no way Scrutor could really have a proper implementation of FromCallingAssembly.

khellang commented 5 years ago

Hmm. There's definitely something weird going on here.

I've made the following test app:

using LibraryA;
using LibraryB;
using System;
using System.Reflection;

// In App.dll

namespace App
{
    public static class Program
    {
        public static void Main(string[] args)
        {
            Console.WriteLine("App");
            Console.WriteLine("Entry: " + Assembly.GetEntryAssembly());
            Console.WriteLine("Executing: " + Assembly.GetExecutingAssembly());
            Console.WriteLine("Calling: " + Assembly.GetCallingAssembly());

            Console.WriteLine();

            Console.WriteLine("Library A");
            Console.WriteLine("Entry: " + A.GetEntryAssembly());
            Console.WriteLine("Executing: " + A.GetExecutingAssembly());
            Console.WriteLine("Calling: " + A.GetCallingAssembly());

            Console.WriteLine();

            Console.WriteLine("Entry (Transitive): " + A.TransitiveGetEntryAssembly());
            Console.WriteLine("Executing (Transitive): " + A.TransitiveGetExecutingAssembly());
            Console.WriteLine("Calling (Transitive): " + A.TransitiveGetCallingAssembly());

            Console.WriteLine();

            Console.WriteLine("Library B");
            Console.WriteLine("Entry: " + B.GetEntryAssembly());
            Console.WriteLine("Executing: " + B.GetExecutingAssembly());
            Console.WriteLine("Calling: " + B.GetCallingAssembly());
        }
    }
}

// In LibraryA.dll

namespace LibraryA
{
    public static class A
    {
        public static Assembly GetCallingAssembly() => Assembly.GetCallingAssembly();

        public static Assembly GetExecutingAssembly() => Assembly.GetExecutingAssembly();

        public static Assembly GetEntryAssembly() => Assembly.GetEntryAssembly();

        public static Assembly TransitiveGetCallingAssembly() => B.GetCallingAssembly();

        public static Assembly TransitiveGetExecutingAssembly() => B.GetExecutingAssembly();

        public static Assembly TransitiveGetEntryAssembly() => B.GetEntryAssembly();
    }
}

// In LibraryB.dll

namespace LibraryB
{
    public class B
    {
        public static Assembly GetCallingAssembly() => Assembly.GetCallingAssembly();

        public static Assembly GetExecutingAssembly() => Assembly.GetExecutingAssembly();

        public static Assembly GetEntryAssembly() => Assembly.GetEntryAssembly();
    }
}

Which gives me the following result:

App
Entry: App, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
Executing: App, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
Calling: App, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null

Library A
Entry: App, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
Executing: LibraryA, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
Calling: App, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null

Entry (Transitive): App, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
Executing (Transitive): LibraryB, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
Calling (Transitive): LibraryA, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null

Library B
Entry: App, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
Executing: LibraryB, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
Calling: App, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null

If you think of your app as App and Scrutor as LibraryA (or LibraryB), the result above tells me that GetCallingAssembly should resolve to your app assembly, i.e. the assembly calling Scrutor's methods. When it comes to GetExecutingAssembly, that seems to always resolve the "current" assembly, i.e. the assembly that the code lives in.

khellang commented 5 years ago

I think the reason why FromCallingAssembly might produce different results in different circumstances is due to (lack of) inlining:

If the method that calls the GetCallingAssembly method is expanded inline by the just-in-time (JIT) compiler, or if its caller is expanded inline, the assembly that is returned by GetCallingAssembly may differ unexpectedly. For example, consider the following methods and assemblies:

  • Method M1 in assembly A1 calls GetCallingAssembly.
  • Method M2 in assembly A2 calls M1.
  • Method M3 in assembly A3 calls M2.

When M1 is not inlined, GetCallingAssembly returns A2. When M1 is inlined, GetCallingAssembly returns A3. Similarly, when M2 is not inlined, GetCallingAssembly returns A2. >When M2 is inlined, GetCallingAssembly returns A3.

This effect also occurs when M1 executes as a tail call from M2, or when M2 executes as a tail call from M3. You can prevent the JIT compiler from inlining the method that calls GetCallingAssembly, by applying the MethodImplAttribute attribute with the MethodImplOptions.NoInlining flag, but there is no similar mechanism for preventing tail calls.

justinbhopper commented 5 years ago

If I'm reading this correctly, you're saying that FromCallingAssembly might return the expected assembly in special circumstances if inlining occurred?

If so, it seems doubtful that inlining would occur under normal circumstances considering how scrutor is typically used, and in any case you certainly can't rely on it to occur. I don't see any way Scrutor could reliably provide a FromCallingAssembly mechanism, unless there's some way to walk the assembly call stack at runtime (ew).

khellang commented 5 years ago

unless there's some way to walk the assembly call stack at runtime (ew).

Yeah, at that point it's much better to just pass the returned assembly from GetCallingAssembly directly.

geirsagberg commented 4 years ago

I agree, GetExecutingAssembly should probably just be removed in the state it is now. It is probably harder to do the wrong thing if you have to specify the assembly directly :)

MisinformedDNA commented 3 years ago

Here's my simple workaround:

services.Scan(s => s.FromAssemblies(Assembly.GetExecutingAssembly())
MisinformedDNA commented 1 year ago

Got stuck on this again. For CallingAssembly, I needed to do

var assembly = Assembly.GetCallingAssembly();
services.Scan(s => s.FromAssemblies(assembly));

Otherwise, the calling assembly ends up being Scrutor.