Issue Details (XML | Word | Printable)

Key: SPR-5863
Type: New Feature New Feature
Status: Open Open
Priority: Major Major
Assignee: Sam Brannen
Reporter: Kristian Rosenvold
Votes: 2
Watchers: 2
Operations

If you were logged in you would be able to see more operations.
Spring Framework

Fix concurrency issues in junit test fixtures (patch enclosed)

Created: 24/Jun/09 05:55 PM   Updated: 02/Mar/10 08:43 AM
Component/s: SpringTEST
Affects Version/s: 3.0 M3
Fix Version/s: 3.1 RC1

Time Tracking:
Not Specified

File Attachments: 1. Text File 1concurrencyFix2051.patch (77 kB)
2. Text File 2tests2051.patch (32 kB)
3. Text File 3dirtiesContext2051.patch (34 kB)
4. Text File completeFix.patch (190 kB)
5. Text File springFailingTest.patch (33 kB)

Environment: Ubuntu, RHEL, windows

Virtual Machine: Sun JVM - 1.5
Platform: Standalone


 Description  « Hide

Newer versions of junit support concurrent running of tests. The org.springframework.test package is really not designed for
concurrency. The enclosed fixes sharpen focus on concurrency (including making mutable state much more distinct from immutable state), increasing separation between data for each test-method run and the class they are being run on.

The patch contains minor changes to the ContextLoaderListener interface.

The patch consists of failing test (patch 1) and fix (patch 2), including several new tests. If you choose to apply the patch with the failing test, you must revert this before applying the fix (failing test is contained in fix patch). The included failing test may not produce concurrency issues (fail) in all cases and on all hardware platforms. They have been known to fail consistently on 3 different machines,
usually upon first run.

Details of the patch:
Classes TestContextManager and TestContext are the biggest changes.

Additionally upon completing the functionality, I had multiple deadlocks in the JVM when running my real test-base. I solved this
by using a JDK1.5 ReaderWriterLock in the "RequestAttributes" getSessionMutex() method. It really looks to me like the creation of this mutex should be moved to one of the loader filters; since it's always created as of this patch.

Additionally, the patch contains a "MockContextLoader" class; this class transfers attributes between threads. I'd really like you guys to check that code out before accepting it; there may be other smarter ways of doing this. It's only a part of the test code-base, but once it's included it sets a standard.

Real-life tests

The patch has been applied to a local version of spring3 that has been running stably with multi-core machines and multi-cpu servers too. We have been running a continious build using both parallel classes, methods and both. This is a full-scale build that was adaptible to multi-threaded test running. The application under tests uses lots of web-scopes etc.

For the documentation:

Parallel running of tests

From version 3.0, spring supports parallel running of tests through the junit4 fixture, SpringJUnit4ClassRunner. Running builds in parallel with
junit is only supported in later versions of junit, and it is recommended to use at least junit 4.6 for this feature. Please also note that there's no guarantee your tests will run properly in parallel, a number of general concurrency issues have to be obeserved when running tests in parallel. Your runner can usually let you choose between classes, methods and both. Classes is usually the easiest to get working, "both" the hardest. All three modes are supported.



Kristian Rosenvold added a comment - 24/Jun/09 05:56 PM

This is the failing test in isolation, can be applied to trunk of spring m3


Kristian Rosenvold added a comment - 24/Jun/09 05:57 PM

The complete patch. Apply this to a CLEAN svn up; do not apply the failing-test patch first. Merged well at rev 1417.


Kristian Rosenvold added a comment - 14/Sep/09 01:55 AM

I have come to understand that I can split this patch in two; I can separate the concurrency issue into a separate issue, thereby reducing the overall footprint of the patch slightly.

The concurrency issue only arises when you have concurrent construction of session scoped artifacts within the same session. In real life this is probably only an issue for quite few applications, and the chances of this occuring are slim. When running parallel tests, this happens all of the time, but can be solved by introducing a non-blocking session scope from within the MockContextLoader instead.

Is there any chance I can make you reconsider the main concurrency fix for 3.0 if I split the patch it in two ? (I'm probably a few months late with this suggestion
The patch for the main concurrency fixes would only affect spring-test.


Kristian Rosenvold added a comment - 05/Oct/09 03:56 AM

Enclosed is an updated version of the concurrency patches that apply cleanly on trunk release 2051. Apply patches within the org.springframework.test module, using patch -p1

Due to the mentioned workaround for deadlock issue wrt Session object, I will be making a separate issue for this problem.

This patch now consists of three files with "2051" in the name. The two first are the main fix for concurrency issues. These two first patches do not support the use of "DirtiesContext" in a concurrent environment. DirtiesContext is supported with the third patch, although this third patch is more of an idea and you may consider accepting this too.

The original "failing test" can still be used as evidence.

The patch is being maintained at http://github.com/krosenvold/spring-test


Kristian Rosenvold added a comment - 02/Mar/10 08:43 AM

Just for the record, all the relevant bits and pieces necessary to support this patch are now released (aspectj 1.6.6), junit 4.8.1 (works from 4.7 but important concurrency bugs are fixed in 4.8.1). maven surefire2.5 support running with concurrent junit. This patch has also been running stably for quite some time in a number of projects.