Scrutor icon indicating copy to clipboard operation
Scrutor copied to clipboard

FromExecutingAssembly and FromCallingAssembly are misleading

Open justinbhopper opened this issue 5 years ago • 9 comments

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:

  • FromCallingAssembly will use the assembly that called Scrutor, rather than the actual calling assembly.
  • FromExecutingAssembly will use Scrutor's assembly. Always.

In other words,

  • scan.FromCallingAssembly === scan.FromAssemblies(Assembly.GetExecutingAssembly())
  • scan.FromExecutingAssembly === scan.FromAssemblies(typeof(Scrutor).Assembly)

justinbhopper avatar Jun 04 '19 00:06 justinbhopper

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? 🤔

khellang avatar Jun 04 '19 06:06 khellang

@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.

justinbhopper avatar Jun 06 '19 03:06 justinbhopper

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 avatar Jun 06 '19 07:06 khellang

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.

khellang avatar Jun 06 '19 07:06 khellang

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).

justinbhopper avatar Jun 08 '19 21:06 justinbhopper

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.

khellang avatar Jun 09 '19 16:06 khellang

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 :)

geirsagberg avatar Sep 10 '19 11:09 geirsagberg

Here's my simple workaround:

services.Scan(s => s.FromAssemblies(Assembly.GetExecutingAssembly())

MisinformedDNA avatar Apr 21 '21 18:04 MisinformedDNA

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.

MisinformedDNA avatar Jul 13 '23 17:07 MisinformedDNA