AOP Pointcuts cannot be relied upon, as they silently fail, breaking their contract. [SPR-8456]

[spring-projects-issues]

nigel magnay opened SPR-8456 and commented

This is probably mildly related to #12238.

I have been converting a legacy application to use <aop: style pointcuts for transaction management, rather than old-style TransactionProxyFactoryBeans.

In the application, an aop pointcut is defined and used to match 2 beans - e.g:
aop:config
<aop:pointcut id="serviceOperation" expression="execution(* net.blah.Service.(..))"/>
<aop:advisor advice-ref="txAdvice" pointcut-ref="serviceOperation" />
..

However, at runtime we find one service is transaction wrapped, but the other is not.

Tracing through the spring code, I find that the wrapper object is, in fact, being successfully created. However, I believe that later on, the first attempt to create the underlying bean fails (possibly down to a BeanInCreation loop exception). When it fails, the proxy is removed from the singleton list - so when it is later eventually successfully created it doesn't get the correct proxy.

I can't say for sure, as I don't really know the spring code, but it seems plausible.

Either way, this is a blocking issue; having transaction aspects silently fail is (usually) quick to identify - but other aspects (logging, security) can be extremely hard to notice when they suddenly become non-operational. At the very least, the container should warn, or fail to start. The wider issue is that the mostly random order in which beans are wired up inevitably contains loops - spring copes with this - but leads to the situation when subtle things like bean name changes or the introduction of another unrelated component can cause total failure.

---

Affects: 3.0.5

Attachments:

• springbug.zip (8.85 kB)

[spring-projects-issues]

Chris Beams commented

Hi Nigel,

This certainly sounds like a valid issue. At this point, however, we'll need at least some detailed stack traces, and preferably a test case or small project that can reproduce the issue. Thanks.

[spring-projects-issues]

nigel magnay commented

Ordering dependent.

[spring-projects-issues]

nigel magnay commented

Sure - one of the problems is of course that we don't really get a stack trace (well, we get a 'not enlisted in transaction' error, but that's the effect of the aspect not being used rather than the actual issue.

I'm trying to recreate it in a minimal test project - ours is very big, and the issue is intermittent (as I suspect it's down to a case when the bean behind the aspect is attempted to be created and fails with somehting like a BeanAlreadyInCreationException). I can put watchpoints in IntelliJ to find out where those kinds of things happen, but they don't generate stack traces. I'm not entirely sure I understand the circumstances where this can happen, but I will try and dig.

One possibility is that, because we've legacy code, it might be down to some mixing of 'old' and 'new' styles. I'm aware of the warning in section 7.3 of the documentation about BeanNameAutoProxyCreator - I don't think we're using this, but the 'advice not being woven' problem makes me think it's a similar issue.

I've attached a project (which was created to try and help this) where there is also a bug whereby the beans get created correctly if they are defined in one order (context3) but bombs out if they are defined in a different order (context4). Don't know if it is related or a separate bug.

[spring-projects-issues]

nigel magnay commented

Ok - I'm still having trouble capturing this as a simple project, but I think I can describe what is happening.

Bean construction is in process. There are many beans, but 2 are significant:

MyService - the service wrapped with AOP style transaction management.
MyManager - a bean that is declared via TransactionProxyFactoryBean (the target being MyManagerTarget).

During this phase, the PatientService AOP wrapper has been created and added to the factory correctly.

The bean factory is constructing beans in the following path:

bean1 -> MyManager -> MyManagerTarget -> [beans...] -> bean2 -> MyService -> [beans....] -> bean3 -> MyManager

At the point where MyManager is attempting to be wired into bean3, a "FactoryBeanNotInitializedException: FactoryBean is not fully initialized yet" exception is thrown. This would seem to be correct.

This exception is caught inside AbstractBeanFactory:297, where 'destroySingleton' is called for 'MyService'. The context here is that MyService is being wired into bean2; I.E: AbstractAutowireCapableBeanFactory::applyPropertyValues is operating for bean2 and trying to getBean(MyService).

In other words
bean2 is being wired
bean2 needs MyService
... bean3 needs MyManager
MyManager is a proxy, it's not ready yet, !exception!
As exception rolls up the chain, destroySingleton is called, removing all the beans from the beanfactory.

When Spring tries next to find a MyService to inject into a bean, the AOP wrapper is no longer there, so it creates a new bean with no AOP instrumentation - hence the bug.

Crucially, the chain [beans...] do not have explicit relationships - I.E: this is not a loop that can be got rid of - statements like

@Resource
IMyInterface myThing;

call getBeanNamesForType() and cause many many beans to be instantiated. This is exacerbated by the fact many times when a ProxyFactoryBean is returned - it /itself/ has not had its' properties set yet, so is unable to determine what type it is proxying, meaning the whole bean (and all it's dependencies, and hence the loop) have to get created. This seems broken to me, and is probably another variant of #12509.

[spring-projects-issues]

Chris Beams commented

Hi Nigel,

First, thanks for the detailed attempt to explain. Even with this in hand, a reproduction test case is probably going to be critical -- it's just a little too speculative right now, and if you're having a hard time reproducing it in the small, chances aren't good I'll be able to. Any possibility you can still get a test case put together? We'd certainly be happy to fix any issue - as long as we can see it of course.

The intermittent nature of the problem is especially troubling. It's not clear to me what would be causing non-determinism in such a case. It's possible that we have a race condition somewhere, but all the more reason that we need a case that has been known to fail at least once.

[spring-projects-issues]

pas filip commented

If you have the luxury of being able to migrate to compile time weaving of the Transactional aspect your proxy worries should be over.