|Sandy McArthur||Mar 26, 2006 11:14 pm|
|Phil Steitz||Mar 27, 2006 6:42 pm|
|robert burrell donkin||Mar 28, 2006 1:25 pm|
|Phil Steitz||Mar 28, 2006 6:32 pm|
|Sandy McArthur||Mar 28, 2006 6:59 pm|
|Peter Steijn||Mar 28, 2006 8:23 pm|
|Craig McClanahan||Mar 28, 2006 8:29 pm|
|Sandy McArthur||Mar 28, 2006 8:51 pm|
|Subject:||Re: Dbcp unit test failures with Pool 1.3 [was: [VOTE] Release Pool 1.3 based on 1.3-rc4]|
|From:||Sandy McArthur (sand...@apache.org)|
|Date:||Mar 26, 2006 11:14:51 pm|
Anyone up for a game of Clue?
I think testThreading did it, with assertTrue in TestConnectionPool.
On 3/26/06, Phil Steitz <phil...@gmail.com> wrote:
-0 Could be a problem with setup or test itself, but with the 1.3-rc4 jar, I am getting failures in [dbcp] test org.apache.commons.dbcp.TestJOCLed. These tests pass with 1.2. The good news is the other [dbcp] test failures that pool 1.3 is supposed to fix succeed for me. Failures below happen on both Sun Linux JDK 1.5.0_06 and 1.4.2_10. <snip/> Checked sigs and contents and everything looks good. Could be test failure is due to bad test in [dbcp]. Will change to +1 when this is resolved.
Testcase: testPooling(org.apache.commons.dbcp.TestJOCLed): FAILED
org.apache.commons.dbcp.TestConnectionPool.testPooling(TestConnectionPool.java:270) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at
Testcase: testConnectionsAreDistinct(org.apache.commons.dbcp.TestJOCLed): Caused
Cannot get a connection, pool error: Timeout waiting for idle object
org.apache.commons.dbcp.SQLNestedException: Cannot get a connection,
pool error: Timeout waiting for idle object
org.apache.commons.dbcp.TestConnectionPool.testConnectionsAreDistinct(TestConnectionPool.java:296) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) Caused by: java.util.NoSuchElementException: Timeout waiting for idle object at
org.apache.commons.pool.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:825) at org.apache.commons.dbcp.PoolingDriver.connect(PoolingDriver.java:175) ... 18 more
This same error occurs for testOpening, testClosing, testMaxActive, testThreaded, testPrepareStatementOptions, testNoRsetClose and testHashCode.
Who did it? How did it happen? Lets look at the clues.
First while we are running TestJOCLed, it subclasses TestConnectionPool and that is where the failures are really happening.
* testPooling: This test method passes when you run it by itself. It fails because while it looks like it's written to handle either a FIFO or a LIFO pool implementation it doesn't if the GenericObjectPool backing Dbcp has more than 2 idle connections. When the test is run after other tests the GOP contains 4 idle connections. With a LIFO the most recently returned is the first one borrowed so it doesn't matter how many idle connections already exist at the start of the test. Since GOP is now a FIFO the test fails because is incorrectly assumes that a recently returned connection will be the next one borrowed. IMO testPooling should be removed as it really testing Pool's behavior and not Dbcp's behavior.
* testConnectionsAreDistinct: This test method passes when you run it by itself. Took me a while to figure out why this fails. It fails because the GenericObjectPool backing Dbcp is configured with a maxActive of 10 but for me the test fails after borrowing 9 successful connections. I'm pretty sure somewhere two of the test methods are borrowing connections and not following the proper contracts and closing their connections.
* testOpening, testClosing, testMaxActive, testThreaded, testPrepareStatemetnOptions, testNoRsetClose, testHashCode: all pass when run individually. They all fail because when testConnectionsAreDistinct fails it doesn't close any connection's it opened in a finally block so the shared GenericObjectPool which has a maxActive set to 10 thinks it's maxed out with 10 borrowed connections (9 were from testConnectionsAreDistinct).
This whole unit test class has a problem in that each test method is not independent of the others. Each test method works when run alone because they have a fresh state that way. It's when one test leaves garbage state around after it runs that is affects other tests and triggers a cascade of failures.
So where is the garbage state coming from? Like I said at the top, testPooling did it with the assertTrue, specifically lines 270 and 271 of TestConnectionPool:
270: assertTrue( underconn3 == underconn || underconn3 == underconn2 ); 271: conn3.close();
When the assertTrue throws an exception the conn3.close is never called resulting in a connection leak.
The easiest fix is to just transpose those two lines and everything else but testPooling passes just fine. That still leaves testPooling as failing and I think that should be solved by removing the test method completely. I think it's fair to say that unit tests for Pool belong in the Pool component code base.
Still, someone should look at the testConnectionsAreDistinct test method as it doesn't clean up after itself when it fails.
Also someone should rework the unit test as a whole so when each test method is run there is no residual state left over from other test methods. Until this is fixed TestConnectionPool isn't really testing what you think it is and probably technically broken.
-- Sandy McArthur
"He who dares not offend cannot be honest." - Thomas Paine