Thread safety issue in InjectionMetaData affecting setter methods annotated with @Resource tags [SPR-7642]

[spring-projects-issues]

Jon Seymour opened SPR-7642 and commented

This issue was detected with Spring 2.5.6, but the same issue exists in Spring 3.0.4

The issue is very similar to the issue reported in 7635, but involves different code paths.

File: org/springframework/beans/factory/annotation/InjectionMetadata.java
Lines: 150-155 (inject), 172-182 (checkPropertySkipping).

Assume a large number of threads are released at the interface of a freshly initialized Spring container containing several prototype bean definitions with @Resource annotations on a setter method.

Suppose that at least two threads enter the inject() method at the same time.

Both threads will see this.skip == null and proceed to call checkPropertySkipping(pvs) at line
151.

Both threads enter checkPropertySkipping. Thread 1 enters the first branch, and evaluates the expression
pvs.contains(this.pd.getName()) and sees false. Thread 1 proceeds to the next if block, evaluats pvs instanceof MutablePropertyValues as true, then calls pvs.registerProcessedProperty. It then exits the method with a return value of false. this.skip is set to false, and the reflective method call is executed (line 159).

At this point Thread 2 resumes. When it evaluates pvs.contains(this.pd.getName()) it sees the value true and therefore takes the true branch of the nested if statement. this.skip is then set to true and the method returns without executing the reflective method call at line 159. All subsequent entrants into this method for this object will see this.skip as true and thus return early from the function without executing the reflective method call.

The net result is that only one instance has its @Resource annotated setter method call and all subsequent instances of the same bean fail to have their dependency injected as expected.

It will be noted that if thread 1 proceeds to completion without interference from Thread 2, then this.skip will be set to false and all instances will be initialized as expected (and, as normally observed, in a environment in which initialization occur gradually).

The obvious solution to this issue is to synchronize the execution of the block of code between lines 150 and 155.

---

Affects: 2.5.6, 3.0.4

Issue Links:

• Unresolved thread safety issue in AutowiredAnnotationBeanPostProcessor.AutowiredMethodElement [SPR-7635] #12291 Unresolved thread safety issue in AutowiredAnnotationBeanPostProcessor.AutowiredMethodElement

Referenced from: commits 7893b3e, ac5b1bc

[spring-projects-issues]

Jon Seymour commented

I rated this minor, however, the issue is at least as serious at 7635, so the maintainers might like to adjust the priorities of this and 7635 accordingly.

[spring-projects-issues]

Juergen Hoeller commented

This should be fixed in tomorrow's 3.0.5 snapshot, with the "contains" call replaced with a "getPropertyValue != null" call which doesn't react to processedProperties (and hence doesn't suffer from a potential side effect with registerProcessedProperty).

Juergen