Race condition in AutowiredAnnotationBeanPostProcessor.AutowiredMethodElement.inject()? [SPR-5658]

[spring-projects-issues]

Mattias Nissler opened SPR-5658 and commented

At work, we've hit a bug in an application that:

a) I have only seen once, so cannot reproduce
b) I'm quite sure is caused by an @Autowired setter called with a null argument (which should be impossible)

When analyzing the problem, we found that three instances of the bean being autowired were instantiated in rapid succession (i.e. concurrently). This made us think of checking whether there maybe concurrency issues within Springs bean initialization code. Ultimately I looked at the code in AutowiredAnnotationBeanPostProcessor.AutowiredMethodElement.inject() and I think I came up with a race condition scenario that does explain the injected null reference:

Consider AutowiredAnnotationBeanPostProcessor.AutowiredMethodElement.inject():

Let's say there are 3 Threads: t1, t2, t3 and the AutowiredMethodElement is in the state cached == false, cachedMethodArguments == null. This means that there probably hasn't been any call to inject() before and checking our log files, we indeed found that we didn't instantiate a bean of this type before. Now what I guess could have happened is:

1. t1 enters, finds cached == false, steps into else block and is interrupted
2. t2 enters, finds cached == false, steps into else block and is interrupted
3. t1 continues, fills in cachedMethodArguments, sets cached = true
4. t3 enters, finds cached == true, steps into if block and is interrupted
5. t2 continues, executes cachedMethodArguments = new Object[...] and is interrupted (chachedMethodArguments contains null references now!)
6. t3 continues, copies cachedMethodArguments[i] to arguments[i] (all null)
7. t3 invokes @Autowired annotated method, thereby injecting the null value

Have I missed any synchronization mechanism that prevents this? If not, it's probably a bug that needs fixing ;-)

---

Affects: 2.5.6

Referenced from: commits a2fdb68

[spring-projects-issues]

Juergen Hoeller commented

Thanks for raising this! There is indeed a potential race condition there... I've revised this with a synchronized block for the non-cached case, which should hopefully be a good tradeoff between efficient access and properly coordinated caching. This will be available in the upcoming 3.0 M3 release.

Juergen

[spring-projects-issues]

Jon Seymour commented

I have added a link to #12291 where I discuss a possible deadlocking issue that might be caused by the resolution of this issue.

This is for information of readers only...