Race condition in AutowiredAnnotationBeanPostProcessor [SPR-11022]

[spring-projects-issues]

Piotr Findeisen opened SPR-11022 and commented

The -AutowiredAnnotationBeanPostProcessor.AutowiredFieldElement and- AutowiredAnnotationBeanPostProcessor.AutowiredMethodElement have double-check pattern and this results in a serious race condition resulting in @Autowired -field-/methods not being autowired.

Explanation

In both -AutowiredFieldElement,- AutowiredMethodElement the volatile boolean cached guards volatile Object[] cachedMethodArguments but when the cachedMethodArguments is populated the order is as follows:

1. synchronize on this
2. set cachedMethodArguments to new array (all fields null)
   • this gets flushed immediatelly, the cachedMethodArguments is volatile
3. populate cachedMethodArguments
   • this is not flushed, as array elements are not volatile
4. set cached = true
   • this gets flushed immediatelly, the cached is volatile
   • and bang! now if some other thread enters the method, it will find cached=true, cachedMethodArguments not null, but its contents may be pretty anything
5. leave synchronization block

solution

• set cachedMethodArguments only after this array is fully created and populated
• or do not use double check pattern

---

Affects: 3.1.2

Referenced from: commits spring-attic/spring-framework-issues@57002b6, spring-attic/spring-framework-issues@da21a32, spring-attic/spring-framework-issues@a6f2f61

[spring-projects-issues]

Piotr Findeisen commented

I've created a repro project, spring-attic/spring-framework-issues#59

[spring-projects-issues]

Piotr Findeisen commented

This looks very similar to #12291, #9607, #10329 but all those issues should be fixed in my version (3.1.2), so I assume this is not a duplicate of those issues.

[spring-projects-issues]

Phil Webb commented

I have merged your repo issue [1] but after upgrading to Spring 3.1.4 it appears to pass. Could you please verify with that release and re-open this issue if you still see problems.

[1] spring-attic/spring-framework-issues@57002b6

[spring-projects-issues]

Piotr Findeisen commented

Phil Webb,

thanks for your prompt reply.

Have you been able to reproduce the bug using 3.1.2 before you updated my repro project?

[spring-projects-issues]

Piotr Findeisen commented

From code reading -- this does not affect AutowiredFieldElement; I've fixed the description.

[spring-projects-issues]

Phil Webb commented

Yes, The repro project you submitted failed on 3.1.2 but not 3.1.4

[spring-projects-issues]

Piotr Findeisen commented

Oh, good, thanks! So I confirm, we see the same.

Could you please change resolution to something more adequate? 'Fixed' perhaps (since bug existed but is fixed)? Or 'Obsolete'?

I'm afraid someone looking at 'Cannot reproduce' resolution may think it was just a bogus bug report and they may not consider this a reason to upgrade to 3.1.4.
(Besides the obvious problem that I link here from internal BTS and I'll have problem explaining why I said my internal bug was caused by Spring)

[spring-projects-issues]

Phil Webb commented

I have marked it as Duplicate since I assume that it was raised and fixed previously.

[spring-projects-issues]

Piotr Findeisen commented

Thanks!

Btw. I see that the only commit between 3.1.2 and 3.1.4 towards ./org.springframework.beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java is this:

• 26ee0c4

It does not change how the cachedMethodArguments array is handled.
So either my initial reasoning was a total crap (quite likely, I presume) or I don't understand why it does occur no longer.

Anyway -- thanks!

(I still will have problem upgrading Spring to 3.1.4 because of #15623)