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

ScheduledAnnotationBeanPostProcessor should reliably apply after AnnotationAwareAspectJAutoProxyCreator [SPR-14692] #19256

Closed
spring-issuemaster opened this issue Sep 12, 2016 · 1 comment

Comments

@spring-issuemaster
Copy link
Collaborator

commented Sep 12, 2016

Mischa Neumann opened SPR-14692 and commented

if you use @EnableAspectJAutoProxy and e.g. @EnableScheduling you might accidentally create a ScheduledMethodRunnable with an unproxied bean.

this is caused by AnnotationAwareAspectJAutoProxyCreator and ScheduledAnnotationBeanPostProcessor both having a default order of Ordered.LOWEST_PRECEDENCE.

as the developer usually has no insight in which post processor is applied in which order and the ordering might be critical for the application to function properly, I would suggest:

  • log all post processors and their ordering at INFO level on startup
  • force all post processors to have a unique order (there is enough space between Ordered.LOWEST_PRECEDENCE and HIGHEST_PRECEDENCE)
  • review the existing spring framework post processors for having a reasonable default order

Affects: 4.2.7

Issue Links:

  • #19526 Ordered interface not respected on non-singleton aspects
  • #19633 SchedulingConfigurer's ScheduledTaskRegistrar should reliably shut down before TaskScheduler

Referenced from: commits fd21e0e, edc62be

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 29, 2016

Juergen Hoeller commented

From my perspective, AnnotationAwareAspectJAutoProxyCreator's default order is alright; our general AOP post-processors are all ordered that way, leaving a custom order open to the user but also allowing for declaration order to be significant (with the same order values). Post-processor ordering is a rather subtle affair overall since the order effectively only applies to each callback: i.e. first all postProcessBeforeInitialization callbacks, then all postProcessAfterInitialization callbacks. I'd rather not make this more complex through implementation-specific default order values that would subtly affect existing declaration-order scenarios.

Solving your scenario in a different way, it's rather ScheduledAnnotationBeanPostProcessor (and its sibling JmsAnnotationBeanPostProcessor) which should always come in a final phase (after the AOP phase) since they're dispatching to the bean. The easiest way to enforce this is for them to declare MergedBeanDefinitionPostProcessor, a marker for internal post-processors which insist on seeing the final runtime bean definition. This rule was introduced in #10734 and has been applied to AutowiredAnnotationBeanPostProcessor and co already but unfortunately not to ScheduledAnnotationBeanPostProcessor yet. I've closed that gap for 4.3.6 now.

As a final note, as far as I could reproduce, @EnableAspectJAutoProxy doesn't actually reveal the problem since the internal auto-proxy creator behind @EnableAspectJAutoProxy will be set up with Ordered.HIGHEST_PRECEDENCE, even if the AnnotationAwareAspectJAutoProxyCreator class LOWEST_PRECEDENCE as a default (like all other AOP post-processor classes). As a consequence, it always comes first, with ScheduledAnnotationBeanPostProcessor reliably seeing the properly proxied instance. However, with a manual AnnotationAwareAspectJAutoProxyCreator bean definition, the problem does show and is being addressed by the fix above now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.