Mailing-List: contact commons-dev-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
Message-ID: <408ED5A6.2AC3B87A@woodfieldfamily.org>
Date: Tue, 27 Apr 2004 15:50:30 -0600
From: Wayne Woodfield <jakarta@woodfieldfamily.org>
MIME-Version: 1.0
To: Jakarta Commons Developers List <commons-dev@jakarta.apache.org>
CC: Dirk Verbeeck <dirk.verbeeck@pandora.be>
Subject: [dbcp][bug 28579] JUnit test
References: <40835DE9.F1E1CA87@woodfieldfamily.org>
 <4085825A.3030802@pandora.be>
Content-Type: multipart/mixed;
 boundary="------------76A20F4BF958F6CA84F3D088"

--------------76A20F4BF958F6CA84F3D088
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

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
> > -------------------------

--------------76A20F4BF958F6CA84F3D088
Content-Type: text/plain; charset=us-ascii;
 name="TestAbandonedObjectPool.java"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="TestAbandonedObjectPool.java"

/*
 * 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();
	}
}


--------------76A20F4BF958F6CA84F3D088
Content-Type: text/plain; charset=us-ascii

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
--------------76A20F4BF958F6CA84F3D088--