JSkimming / Castle.Core.AsyncInterceptor

Library to simplify interception of asynchronous methods
Apache License 2.0
295 stars 42 forks source link

"proceed.Invoke(invocation)" calls again "AsyncInterceptorBase.InterceptAsync" #117

Closed fharr closed 3 years ago

fharr commented 3 years ago

I am using IAsyncInterceptor with the built-in .Net Core (3.1) IServiceCollection to inject transactional behavior around my services and I am facing a very strange issue:

For some services, but not all of them (despite they are all registered the same way), in my implementation of AsyncInterceptorBase.InterceptAsync, the call to proceed.Invoke(invocation); internally recall InterceptAsync, as you can see on this debugger screenshot:

image

Here is the associated call stack:

image

Here is how I register my services:

public static IServiceCollection AddProxy<TInterface, TImplementation>(this IServiceCollection services, ServiceLifetime lifetime)
{
    // Register the service implementation
    services.Add(new ServiceDescriptor(typeof(TImplementation), typeof(TImplementation), lifetime));

    // Register the mapping interface <-> implementation
    services.Add(new ServiceDescriptor(typeof(TInterface), serviceProvider =>
    {
        // Get a new interceptor instance
        var interceptor = serviceProvider.GetRequiredService<EndonextAsyncInterceptor>();

        var hasTransactionAttributes = typeof(TImplementation)
            .GetMethods()
            .SelectMany(m => m.GetCustomAttributes(typeof(TransactionAttribute), false))
            .Any();

        // Inject the DbContext if necessary
        if (hasTransactionAttributes)
        {
            interceptor.DbContext = serviceProvider.GetRequiredService<EndonextDbContext>();
        }

        // Get the proxy generator and the service implementation instances
        var proxyGenerator = serviceProvider.GetRequiredService<ProxyGenerator>();
        var actual = serviceProvider.GetRequiredService<TImplementation>();

        // Return the proxy
        return proxyGenerator.CreateInterfaceProxyWithTarget(typeof(TInterface), actual, interceptor);
    }, lifetime));

    return services;
}

IServiceCollection services;
services
    .AddTransient<AsyncInterceptor>()
    .AddSingleton<ProxyGenerator>()
    .AddWithProxy<IService, Service>(ServiceLifetime.Scoped);

Here is the way I get my service instance:

using (var scope = serviceProvider.CreateScope())
{
    var service = scope.ServiceProvider.GetRequiredService<IService>();

    await service.MyCallAsync();
}

And here is my simplified implementation of the IAsyncInterceptor:

public class AsyncInterceptor : AsyncInterceptorBase
{
    private readonly ILogger<AsyncInterceptor> _logger;

    public AsyncInterceptor(ILogger<AsyncInterceptor> logger)
    {
        _logger = logger;
    }

    protected override async Task InterceptAsync(IInvocation invocation, Func<IInvocation, Task> proceed)
    {
        LogEntry(invocation);

        bool inError = false;

        try
        {
            await this.BeginTransactionAsync(invocation);

            await proceed.Invoke(invocation);
        }
        catch (Exception)
        {
            inError = true;
            throw;
        }
        finally
        {
            await this.CommitTransactionAsync(invocation, inError);
        }

        LogExit(invocation);
    }
}

I only use IInvocation in the BeginTransaction and CommitTransaction for reflection purpose (getting an attribute associated to the invoked method).

I did not found any obvious difference between my services declaration, implementation nor, registration.

Do you have an idea of what is going on?

fharr commented 3 years ago

This bug is very strange. There is no issue if I change the order of execution of my calls.

Let me explain: the application I am working on is an ASP.NET Core Web Api (3.1). It has two HostedServices. Let's call them HostedServiceA and HostedServiceB and consider that HostedServiceA is registered first (so it will run first).

When HostedServiceA starts, it retrieves an instance of a ServiceA and call a method:

using (var scope = serviceProvider.CreateScope())
{
   var serviceA = scope.ServiceProvider.GetRequiredService<IServiceA>();

   await serviceA.CallAsync();
}

That call is intercepted and everything works fine.

Then HostedServiceB starts, it retrieves an instance of a ServiceB and call a method:

using (var scope = serviceProvider.CreateScope())
{
   var serviceB = scope.ServiceProvider.GetRequiredService<IServiceB>();

   await serviceB.CallAsync();
}

Then I fall into the issue described.

If I unregister the HostedServiceA (so it doesn't start) or if I invert HostedServiceA and HostedServiceB registration (so HostedServiceB starts first), there is no issue with the call to serviceB.CallAsync().

JSkimming commented 3 years ago

@fharr What version are you using? If it's 1.7 or lower can you upgrade to 2.0.21-alpha? I suspect that may solve your issues.

2.0.21-alpha is no more or less robust, in fact, given the download numbers, it's probably more battle-hardened than others.

I'm going to release 2.0 soon, I've been waiting until .NET 5 to prove all the nullable reference types changes work. The 2.0 release is almost the same as the alpha, and should probably upgrade without needing any code changes.

fharr commented 3 years ago

Yes I am using the 1.7.0 version. I migrated to 2.0.21-alpha and it apparently solves the case, no matter the order of execution of my HostedServices. It's good for me !

Thank you for this great package by the way ! Can't wait for the 2.0 version :D.

JSkimming commented 3 years ago

Thanks for the kind words @fharr, it's always appreciated.

ndrwrbgs commented 3 years ago

Looks related to some of the things you fixed for me so the upgrade fixing it makes sense to me!

On Fri, Dec 18, 2020, 4:05 AM James Skimming notifications@github.com wrote:

Closed #117 https://github.com/JSkimming/Castle.Core.AsyncInterceptor/issues/117.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JSkimming/Castle.Core.AsyncInterceptor/issues/117#event-4129039071, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACSHCOUEPNYU3DLGIIVFPY3SVMSOFANCNFSM4VAEP2OQ .