Issue SPR-368 (AOP proxy creation breaks property initialisation) not fixed in 1.1.2 [SPR-472]

[spring-projects-issues]

Erwin Vervaet opened SPR-472 and commented

See issue #5098, which should be reopened.

Erwin

---

Affects: 1.1.2

[spring-projects-issues]

Juergen Hoeller commented

Rob, can you please have a look at this, at your earliest convenience?

Juergen

[spring-projects-issues]

Rob Harrop commented

After looking into this issue a little bit more I can see where the possible arises. The fix in 1.1.2 only half fixes this problem in that if the method(s) called by the constructor are advised then the behavior is as expected. However, if the method is unadvised then the property values will be overwritten as detailed in the bug report.

The reason for this is that, internally, the Cglib2AopProxy class uses a CGLIB Dispatcher for unadvised methods rather than a MethodInterceptor. A Dispatcher sends calls directly to the proxy target whilst a MethodInterceptor can perform some processing either side of the invocation and thus can check to see the construction state of the proxy and decide whether or not to invoke the call on the target.

A solution for the interim is to make sure that methods that are called by the constructor are advised. In the case of the example given in #5098 that means extending the pointcut regex pattern to cover the setA() method.

There are a few ways to fix this problem:

1. Remove use of Dispatchers altogether
2. Create a dummy object that Dispatchers can re-route to during construction
3. Mandate that all methods called from a constructor must be advised
4. Save target state before proxy creation and then restore afterwards

I don't really favour approach 1 as the Dispatchers give a nice performance boost on unadvised methods. Approach 2 will have the least impact on end users but will result in a new object being created for every class that is proxied. Approach 3 is simple but I can't think of a way to check what methods are called during a constructor so that we can check in code that they are advised. Approach 4 seems like a good one, but not sure how we can reliably save state and restore it when some state may not be exposed externally.

I'll post a link to this issue on the CGLIB dev list and see if they have any ideas. In meantime all comments would be welcome.

Rob

[spring-projects-issues]

Rob Harrop commented

Had a response from Chris Nokleberg at CGLIB regarding this issue:

Hi Rob,

Probably #2 ("Create a dummy object that Dispatchers can re-route to
during construction") is the simplest solution, but I don't like it
either. It is probably better for me to add an option to Enhancer:

e.setInterceptCallsFromConstructor(false);

We actually have to jump through some hoops to intercept those calls in
the first place, so I hope it won't be hard to ignore them. You could
then also get rid of the code you added for the previous issue.

I'll look into it tomorrow (3am here now...)

Chris

Looks like we will get a solution at the CGLIB level which is best all round. In the meantime making any methods called from the constructor advised will remedy this issue.

Rob

[spring-projects-issues]

Erwin Vervaet commented

Great!

But just wondering, since the issue is easy to reproduce shouldn't there have been a unit test to verify that is was really fixed in 1.1.2?

Erwin

[spring-projects-issues]

Rob Harrop commented

There was, but due to an error on my part it wasn't comprehensive enough. In the test I had the method called in the constructor was advised and in the test your supplied it wasn't. I apologise.

Rob

[spring-projects-issues]

Juergen Hoeller commented

I'm closing this for the time being, as we've reopened #5098.

Juergen