This JUnit test will test the fix for bug 28579. It takes between 1-2 seconds to complete on my computer. Even though it simulates a threading issue, it's very reliable. I haven't been able to get it to generate a false positive yet. If you commit this new test to CVS, please insert a line for this test into the org.apache.commons.dbcp.TestAll class. I haven't done so yet.
By the way, I often use tab characters for indentation, and so I have done so with this file. If it is the jakarta or commons standard to use spaces for indentation, I'll use that convention from now on. Look it over -- if any questions or comments, let me know. Wayne Woodfield Dirk Verbeeck wrote: > Hi Wayne, > > Excellent description and patch. The commons-dev mailing list is > indeed the place to start for getting your fix applied. Now a bugzilla > issue needs to be created to record the issue+patch and then I will > apply your patch to DBCP. > Do you have a way to simulate the error? Threading issues are always > difficult to reproduce and your solution is obviously correct but > having a JUnit test is always a plus. > > Thanks > Dirk > > Wayne Woodfield wrote: > > I'm absolutely new to open-source development and groups like this, so > > forgive me if I'm not following the correct protocol, but I found and > > fixed a bug in DBCP and would like to contribute my patch back to the > > project. I've created a patch file and attached it to this e-mail. If > > a comitter to commons-dbcp would please look at my patch as well as the > > explanation below, that'd be great. If you'd like me to do anything > > differently next time I have a patch to offer, just let me know :-) > > > > Wayne Woodfield > > -------------------------
/* * Copyright 1999-2004 The Apache Software Foundation. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ package org.apache.commons.dbcp; import java.util.Vector; import java.util.NoSuchElementException; import junit.framework.Test; import junit.framework.TestCase; import junit.framework.TestSuite; import org.apache.commons.pool.PoolableObjectFactory; import org.apache.commons.pool.impl.GenericObjectPool; /** * @author Wayne Woodfield * @version */ public class TestAbandonedObjectPool extends TestCase { private AbandonedObjectPool pool = null; public TestAbandonedObjectPool(String testName) { super(testName); } public static Test suite() { return new TestSuite(TestAbandonedObjectPool.class); } public void setUp() throws Exception { super.setUp(); AbandonedConfig config = new AbandonedConfig(); config.setRemoveAbandoned(true); config.setRemoveAbandonedTimeout(1); pool = new AbandonedObjectPool(new SimpleFactory(), config); } public void tearDown() throws Exception { super.tearDown(); pool.close(); pool = null; } /** * Tests fix for Bug 28579, a bug in AbandonedObjectPool that causes numActive to go negative * in GenericObjectPool */ public void testConcurrentInvalidation() throws Exception { final int POOL_SIZE = 30; pool.setMaxActive(POOL_SIZE); pool.setMaxIdle(POOL_SIZE); pool.setWhenExhaustedAction(AbandonedObjectPool.WHEN_EXHAUSTED_FAIL); // Exhaust the connection pool Vector vec = new Vector(); for (int i = 0; i < POOL_SIZE; i++) { vec.add(pool.borrowObject()); } // Abandon all borrowed objects for (int i = 0; i < vec.size(); i++) { ((PooledTestObject)vec.elementAt(i)).setAbandoned(true); } // Try launching a bunch of borrows concurrently. Abandoned sweep will be triggered for each. final int CONCURRENT_BORROWS = 5; Thread[] threads = new Thread[CONCURRENT_BORROWS]; for (int i = 0; i < CONCURRENT_BORROWS; i++) { threads[i] = new ConcurrentBorrower(vec); threads[i].start(); } // Wait for all the threads to finish for (int i = 0; i < CONCURRENT_BORROWS; i++) { threads[i].join(); } // Return all objects that have not been destroyed for (int i = 0; i < vec.size(); i++) { PooledTestObject pto = (PooledTestObject)vec.elementAt(i); if (pto.isActive()) { pool.returnObject(pto); } } // Now, the number of open connections should be 0 assertTrue("numActive should have been 0, was " + pool.getNumActive(), pool.getNumActive() == 0); } class ConcurrentBorrower extends Thread { private Vector _borrowed; public ConcurrentBorrower(Vector borrowed) { _borrowed = borrowed; } public void run() { try { _borrowed.add(pool.borrowObject()); } catch (Exception e) { // expected in most cases } } } class SimpleFactory implements PoolableObjectFactory { public Object makeObject() { return new PooledTestObject(pool); } public boolean validateObject(Object obj) { return true; } public void activateObject(Object obj) { ((PooledTestObject)obj).setActive(true); } public void passivateObject(Object obj) { ((PooledTestObject)obj).setActive(false); } public void destroyObject(Object obj) { ((PooledTestObject)obj).setActive(false); // destroying an object will often take a few milliseconds, especially when the object is a network connection try { Thread.sleep(30); } catch (InterruptedException e) { } } } } class PooledTestObject extends PoolableConnection { private boolean active = false; private int _hash = 0; private boolean _abandoned = false; private static int hash = 1; public PooledTestObject(GenericObjectPool pool) { super(null, pool); _hash = hash++; } public synchronized void setActive(boolean b) { active = b; } public synchronized boolean isActive() { return active; } public int hashCode() { return _hash; } public void setAbandoned(boolean b) { _abandoned = b; } protected long getLastUsed() { if (_abandoned) { // Abandoned object sweep will occur no matter what the value of removeAbandonedTimeout, // because this indicates that this object was last used decades ago return 1; } else { // Abandoned object sweep won't clean up this object return 0; } } public boolean equals(Object obj) { if (!(obj instanceof PooledTestObject)) return false; return obj.hashCode() == hashCode(); } }
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
