New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async Interceptor/Invocation support #107

Closed
brunoblank opened this Issue Sep 3, 2015 · 8 comments

Comments

Projects
None yet
2 participants
@brunoblank
Copy link

brunoblank commented Sep 3, 2015

Hi,
I have been investigeting (tesing) the possibility to add support for async/await.

New method on ProxyGenerator:

public TInterface CreateAsyncInterfaceProxyWithoutTarget<TInterface>(params IInterceptorAsync[] interceptors)
        where TInterface : class

Where IInterceptorAsync has one small difference:

public interface IInterceptorAsync
{
    Task InterceptAsync(IInvocationAsync invocation);
}

and IInvocationAsync:

public interface IInvocationAsync
{
    ... 

    Task ProceedAsync();

    ...
}

In order for this to work all methodson the intercepted interface must return Task or Task < T > - this is a limitation that I think is accepted.

To generate IL for an async method is almost "impossible". So this is what I been thinking.

  1. Async method does not support ref or out so the IL can be simplified a bit.
  2. The method must return Task or Task < T >

The AbstractInvocation gets two "helper" methods:

    public async Task ProceedTask()
    {
        await ProceedAsync();
    }

    public async Task<TReturn> ProceedTaskReturn<TReturn>()
    {
        await ProceedAsync();
        return await (Task<TReturn>)ReturnValue;
    }

When generating the IL can check the return type of the method and call and return the result of the helper method like this:

IL_0028:  call       instance class [mscorlib]System.Threading.Tasks.Task`1<!!0> [Castle.Core]Castle.DynamicProxy.AbstractInvocation::ProceedTaskReturn<string>()
IL_002d:  ret

Code to generate:

        ExpressionStatement proceedStatement;
        if (emitter.ReturnType == typeof(Task))
        {
            proceedStatement = new ExpressionStatement(new MethodInvocationExpression(invocationLocal, InvocationMethods.AsyncProceedTask));
        }
        else if (emitter.ReturnType.IsGenericType && emitter.ReturnType.GetGenericTypeDefinition() == typeof(Task<>))
        {
            proceedStatement = new ExpressionStatement(new MethodInvocationExpression(invocationLocal, InvocationMethods.AsyncProceedTaskReturn.MakeGenericMethod(emitter.ReturnType.GenericTypeArguments[0])));
        }
        else
        {
            throw new GeneratorException("Return Type must be of type Task or Task<>");
        }
        emitter.CodeBuilder.AddStatement(proceedStatement);

Is this something that could be added?

Regards
Bruno

@brunoblank

This comment has been minimized.

Copy link

brunoblank commented Sep 6, 2015

Created a PR with first part of async: Added support for async #108

Regards
Bruno

@brunoblank brunoblank closed this Sep 6, 2015

@brunoblank brunoblank reopened this Sep 6, 2015

@kkozmic

This comment has been minimized.

Copy link
Contributor

kkozmic commented Sep 6, 2015

Hi Bruno.

async/await is merely a compiler trick (like iterators and dynamic and unlike say, generics. What would be some scenarios where the current featureset of DP is insufficient to handle them?

The change you're proposing is fairly large and I'd like to make sure it's absolutly necessary before we dive in and spend too much time on it.

Cheers
Krzysztof

@brunoblank

This comment has been minimized.

Copy link

brunoblank commented Sep 6, 2015

Hi Krzysztof,

I have added a small example interceptor to show the case that I have had problem with (this code works, but I have not been able to achive it with current implementation without add .Wait() which blocks that thread).

    public class MyInterceptorAsync : IInterceptorAsync
    {
        public async Task InterceptAsync(IInvocationAsync invocation)
        {
            try
            {
                await invocation.ProceedAsync();
                var target = await (Task<string>)invocation.ReturnValue;
                invocation.ReturnValue = Task.FromResult(target + " - interceptor");
            }
            catch (Exception ex)
            {
                invocation.ReturnValue = Task.FromResult(ex.Message);
            }
        }
    }

If the target is async, and as soon as it hits the first await it rolls out from Proceed. And it is not possible from the interceptor to change the return value (act on exception).

Thanks
/Bruno

@kkozmic

This comment has been minimized.

Copy link
Contributor

kkozmic commented Sep 7, 2015

Can you build a test on existing codebase and point out where it fails you expectations?

@brunoblank

This comment has been minimized.

Copy link

brunoblank commented Sep 7, 2015

This is a failing test on existing codebase.

The problem is in the interceptor where I want to observe the result of the async method to be able to change the return value.

[TestFixture]
public class AsyncInterceptorTestCase
{
    [Test]
    public async Task AyncTestCase()
    {
        var result = new ProxyGenerator().CreateInterfaceProxyWithTarget<IAnInterface>(new AnTarget(), new MyInterceptor());
        Assert.IsNotNull(result);

        var returnValue = await result.AMethod();
        Assert.AreEqual("Oops", returnValue);
    }

    public class MyInterceptor : IInterceptor
    {
        public void Intercept(IInvocation invocation)
        {
            invocation.Proceed();
            var returnValue = (Task<string>)invocation.ReturnValue;
            returnValue.ContinueWith(t =>
            {
                Console.WriteLine(t.Exception.Message);
                invocation.ReturnValue = Task.FromResult("Oops");
            }, TaskContinuationOptions.OnlyOnFaulted);
        }
    }

    public interface IAnInterface
    {
        Task<string> AMethod();
    }

    public class AnTarget : IAnInterface
    {
        public async Task<string> AMethod()
        {
            await Task.Delay(100);
            throw new Exception("Something failed, in target method");
        }
    }

/Bruno

@kkozmic

This comment has been minimized.

Copy link
Contributor

kkozmic commented Sep 8, 2015

invocation.ReturnValue = returnValue.ContinueWith(t =>
{
    Console.WriteLine(t.Exception.Message);
    return "Oops";
}, TaskContinuationOptions.OnlyOnFaulted);

This works

@brunoblank

This comment has been minimized.

Copy link

brunoblank commented Sep 8, 2015

Yes, that works!

After analyzing the async code that my interceptor was generating, I could pin point the flaw. I was able to make a really nice solution to my problem thanks to the knowledge I picked up doing this PR.

There would be no real need to merge this PR as it is possible to work around it. However, I do think having async support in the Interceptor would be useful.

Thank you for spending time on this
/Bruno

@kkozmic

This comment has been minimized.

Copy link
Contributor

kkozmic commented Sep 8, 2015

No worries. I'm glad you solved it :)

Feel free to close the PR and this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment