Open dilandau2001 opened 2 months ago
Hey @dilandau2001, I have reviewed your report and dug dipper into it and I must admit this a design flaw in a way ref/out params are deconstructed and given as object[] to an Around method. The expectation (wrong) was that the Around injection can replace ref for the time of execution and then redirect the result to a proper refs once the method is finished without the original caller ever notice that.
Something like orig call passing ref
-> around infrastructure substitutes the ref
-> aspect user code can manipulate object[] args
-> original method
-> around infrastructure substitutes the ref on the way back
-> original caller has its ref properly working
The problem is (now) obviously that changes to a field passed by ref won't be recorded until after the original method and whole aspect chain is completed. Which wasn't a problem for mere variables passed by ref.
As I said, this is a design flaw, there is no fix (as far as I can tell, It'd require a complete rewrite and changes to API that deals with arguments), I'll need to record this issue in the docs.
Thanks and I'm sorry
Fine. I see using the "Around" method won't be possible. But, in order to achieve my real objective, it would be enough to have some type of "context" that I could use to share the stopwatch instance between the begin aspect and the end aspect. Do you think this other approach could be possible?
It is technically possible, yes, but requires some major redesign. I might have a look at this at a later time
On Tue, 10 Dec 2024, 09:41 dilandau2001, @.***> wrote:
Fine. I see using the "Around" method won't be possible. But, in order to achieve my real objective, it would be enough to have some type of "context" that I could use to share the stopwatch instance between the begin aspect and the end aspect. Do you think this other approach could be possible?
— Reply to this email directly, view it on GitHub https://github.com/pamidur/aspect-injector/issues/235#issuecomment-2529689791, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7HZUHSIJCMTKTFDGIELYD2EYMDHAVCNFSM6AAAAABO4GNBLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRZGY4DSNZZGE . You are receiving this because you commented.Message ID: @.***>
Environment (please complete the following information):
Describe the bug When using a Advice(Kind.Around) to meassure time consumed by methods. When method sets properties passed by reference. Probably it might happen also with out references. When the ref property is set, the change does not occur until the whole advice finishes.
To Reproduce Create a class like this:
Create the aspect like this:
And create a unit test like this:
This unit tests works fine if LogCall is commented out, and it fails otherwise in this assertion: Assert.AreEqual(5, result); When paremeters are passed by reference or out, calling methodInvocation(args) is not really updating at the moment you would expect. It is like if a copy of the value is the one being passed but it does update when the advice finishes because this passes: Assert.AreEqual(5, testedClass.Property);
Additional context What I am trying to create is an aspect that is able to meassure how long it takes for each method to complete its work, for performance analysis. So far I have tried 2 approachs. First I tried to use Advice.Before and Advice.After, but I found no way to share the stopwatcher in a thread safe way. Also this approach does not work when there is an exception within the method. We lack a [Argument(Source.Context)] that could be used as a dictionary to store objects, so we can store an instance in the Before, and use it in the after. Also After should fire when theere is an exception in the method. Then I tried the advice.Around, which look promising but was failing in runtime with many nullreference exceptions. I guess the aspect should be able to support a type of Source of type MethodDelegate, so you can tell it to just proceed, instead of invoking it though the Func<object[], object>(object[] arguments)