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

Subclasses of ApplicatonObjectSupport can no longer be proxied via CGLIB [SPR-2237] #6927

Closed
spring-projects-issues opened this issue Jul 4, 2006 · 4 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

Chris Lee opened SPR-2237 and commented

The latest nightly build (2.0RC2 b69) fails to proxy beans whose implementation inherits from ApplicationObjectSupport.

With RC1, the following messages were logged:

10:36:58.469 INFO [Cglib2AopProxy] Unable to proxy method [public final org.springframework.context.ApplicationContext org.springframework.context.support.ApplicationObjectSupport.getApplicationContext() throws java.lang.IllegalStateException] because it is final: All calls to this method via a proxy will be routed directly to the proxy. [] [main]
10:36:58.469 INFO [Cglib2AopProxy] Unable to proxy method [public final void org.springframework.context.support.ApplicationObjectSupport.setApplicationContext(org.springframework.context.ApplicationContext) throws org.springframework.beans.BeansException] because it is final: All calls to this method via a proxy will be routed directly to the proxy. [] [main]

With RC2b69, an exception is thrown with the following message:

Cannot proxy class 'bridges.facade.aa.AuthenticationFacadeImpl' since it has one or more public final methods.

...it looks like the implementation of Cglib2AopProxy.doValidateClass changed between RC1 & RC2b69.

One workaround would be to use JDK proxies, which works for this specific object; however, we have a number of legacy objects (now Spring-proxied for txns), which is why CGLIB proxying is enabled.


Affects: 2.0 RC2

@spring-projects-issues
Copy link
Collaborator Author

Chris Lee commented

It looks like the fix for http://opensource.atlassian.com/projects/spring/browse/SPR-2163 causes failure when trying to proxy classes with final methods.

Some possible solutions:

  1. Modify ApplicationObjectSupport to remove final modifier on public methods, allowing it to be a CGLIB proxy-able superclass.
  2. Modify Cglib2AopProxy to log final methods as a warning (was info in RC1) instead of throwing an exception.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

The problem here is that proxying final methods via CGLIB methods causes really bad effects: It's the proxy instance itself that handles the call then, based on the proxy instance's state which is completely uninitialized! This only works out of sheer luck (if at all), or if you're not calling that method through the proxy in the first place. In any case, it is an inherent danger, which is why we switched to throwing an exception in this case.

That said, I do appreciate that such proxying works in your case because it only affects configuration methods which will never be called through the proxy in the first place. This scenario might be common enough to warrant switching to warn messages again in case of proxying final methods. Unfortunately, we can't clearly differentiate between configuration methods and operation methods there, so would have to accept the proxying in any case...

I guess the most important lesson to be learned from this is: Proxy with interfaces wherever you can. Only use CGLIB proxies if you really have no way around. For legacy objects that need to have transactions applied, you could for example build a thin wrapper that exposes a service interface and does the transactional proxying at that level...

Rob, what do you think?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Chris Lee commented

Thanks Juergen; good explanation.

We had always planned to convert to proxied interfaces, just not so suddenly :(

I agree that it is important for proxies to 'just work' - and flagging an error for proxying final methods aligns nicely with that.

In the interest of a smooth transition - perhaps a compromise solution can be considered:

<bean class="org.springframework.aop.framework.autoproxy.DefaultAdvisorAutoProxyCreator">
<property name="proxyTargetClass" value="true" />
<property name="proxyFinalMethods" value="true"/>
</bean>
...where the proxyFinalMethods functionality is 'false' by default, heavily documented as to the drawbacks, and logs a large DEPRECATED warning during context startup (and s/b removed in a near-future Spring release)

@spring-projects-issues
Copy link
Collaborator Author

Rob Harrop commented

The simplest and safest solution here is to simply WARN on final methods which I have reverted the code to.

Rob

@spring-projects-issues spring-projects-issues added type: bug A general bug in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 10, 2019
@spring-projects-issues spring-projects-issues added this to the 2.0 RC2 milestone Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

1 participant