CachedIntrospectionResults Not Thread Safe [SPR-9584]

[spring-projects-issues]

elliot barlas opened SPR-9584 and commented

My Spring web application invokes org.springframework.beans.BeanUtils.getPropertyDescriptors in a multi-threaded context. After recent code refactoring, I started noticing something strange: the call occasionally returns an incomplete set of property descriptors. The code change must have produced paths that randomly execute getPropertyDescriptors at exactly the same time. I have written a targeted Java app to demonstrate the problem.

Attached is Main.java, a simple Java app that lines up threads and then invokes getPropertyDescriptors in parallel.

The beans are modeled after my application beans.

When I run the app continuously, failures eventually occur:
for i in {1..1000}; do java -cp <jar path> test.Main 50; done
Fail: 8
Fail: 3
Fail: 8
...

I haven't yet taken the time to identify the thread-safety issue.

---

Affects: 3.1 GA

Attachments:

• Main.java (1005 bytes)
• One.java (253 bytes)
• Three.java (440 bytes)
• Two.java (848 bytes)

Issue Links:

• Overhaul non-void JavaBean write method support [SPR-10029] #14663 Overhaul non-void JavaBean write method support

Referenced from: commits spring-attic/spring-framework-issues@96899fe, spring-attic/spring-framework-issues@16d97dc

2 votes, 3 watchers

[spring-projects-issues]

elliot barlas commented

One point that I missed: making verifyDescriptors synchronized solves the problem.

[spring-projects-issues]

elliot barlas commented

Any reason this hasn't received much attention? This is a critical bug which caused serious problems in my web application.

[spring-projects-issues]

Chris Beams commented

Hi Elliot,

Could you quickly verify that you get the same results against the latest 3.2.0.BUILD-SNAPSHOT artifacts (available from http://repo.springsource.org/snapshot)? There have been a number of changes to CachedIntrospectionResults and it would be good to confirm you're seeing the same behavior.

[spring-projects-issues]

elliot barlas commented

Yes, I'll try this now...

[spring-projects-issues]

elliot barlas commented

After many attempts, I was not able to reproduce the issue with version 3.2.0.BUILD-SNAPSHOT. I guess it's fixed?

[spring-projects-issues]

Chris Beams commented

Hi Elliot, thanks for verifying that. I had a strong suspicion that the changes made in #14357 would have eliminated the problem, and it looks like they did.

The short story there is that #14357 removes ExtendedBeanInfo from the code path for 99% (perhaps more) of all use cases of CachedIntrospectionResults. This means that there may well be a concurrency issue in ExtendedBeanInfo, but that it has been neutralized for almost all users.

Note that #14357 is not being backported to the 3.1.x line, which means that this issue could still crop up for users there, but at this time, I'm marking the issue won't fix because (a) no one else has reported it yet, i.e. it's probably an unlikely scenario, (b) it's fixed in 3.2, (c) any fix to ExtendedBeanInfo would probably represent more trouble than it's worth in terms of committer time and effort.

[spring-projects-issues]

Chris Beams commented

A general note to watchers of ExtendedBeanInfo-related issues: #14663 is a major refactoring of ExtendedBeanInfo and overall support for non-void returning setter methods.

If you have submitted a reproduction project with this issue, we have run it through its paces against these new changes, but we would like to ask you to do the same against your actual applications.

Please consider updating your dev or test builds to work against 3.1.4.BUILD-SNAPSHOT and/or 3.2.0.BUILD-SNAPSHOT to verify, and we would appreciate any feedback, even if it's to let us know that all is well. So that we can consolidate feedback, please add your comments to #14663, and mention the original issue(s) that you were watching.

Thanks!

Note also that testing against 3.1.4 is preferable to 3.2.0 because ExtendedBeanInfo is always in the code path in the latter, while in 3.2.0 we've optimized things such that ExtendedBeanInfo is only in play for bean classes that have one or more non-void returning setter methods.