Skip to content
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

Test demonstrating Issue #25 - Root Cause: Calls to proceed after the first await are intercepted again #26

Closed
wants to merge 2 commits into from

Conversation

ndrwrbgs
Copy link
Collaborator

@ndrwrbgs ndrwrbgs commented Nov 4, 2017

Issue #25

It appears to be related to Castle.Core's AbstractInvocation's synchronous method Proceed().

This method uses a currentInterceptorIndex to tell when it's through all of the interceptors, and not utilize interception (instead using InvokeMethodOnTarget()). In the async case, this method is not being called twice nested (once to trigger the interceptor, and then again to invoke on the target at proceed()), but instead it's being called serially (either creating a new AbstractInvocation or it's exiting the finally in between the calls, allowing currentInterceptorIndex to reset). Therefore, Castle treats the proceed() as an interceptable call.

@ndrwrbgs ndrwrbgs changed the title Test demonstrating Issue 25 - Root Cause: Calls to proceed after the first await are intercepted again Test demonstrating Issue #25 - Root Cause: Calls to proceed after the first await are intercepted again Nov 4, 2017
@JSkimming
Copy link
Owner

@ndrwrbgs I've spent a few hours on this one today, it's proving pretty gnarly. I'm digging into AbstractInvocation as you also must have.

I don't have any more time I'm afraid, I'll try and find to time to investigate further, but I can give no guarantees.

@JSkimming
Copy link
Owner

Fixed with #54

@JSkimming JSkimming closed this Jan 25, 2021
@ndrwrbgs
Copy link
Collaborator Author

ndrwrbgs commented Jan 28, 2021

If it's fixed, we should be able to merge this in now, right?
Ah, I see the commit included its own tests. Let me run this real quick and make sure that it also passes just in case!

@ndrwrbgs
Copy link
Collaborator Author

@JSkimming when I try running the repro test against master, it still produces an infinite interception. This seems to suggest that #54 didn't solve the failure in #26, demonstrating the issue from #25. Could you help me to figure out what I'm missing?

@ndrwrbgs ndrwrbgs reopened this Jan 28, 2021
@ndrwrbgs
Copy link
Collaborator Author

It's helpful when you (me) know how to work (I do not) git and upstreams and actually pull the changes you're trying to validate, when you're trying to validate them.

@ndrwrbgs ndrwrbgs closed this Jan 28, 2021
@JSkimming
Copy link
Owner

@ndrwrbgs I use a bunch of aliases to simplify the workflow, they're largely based on this post by @haacked

I maintain this .gitconfig gist that I copy to all new machines I start using.

https://gist.github.com/JSkimming/9210830

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants