From 66f7a4818f8c62e11dd1e93190e687495c33df25 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 25 Aug 2012 14:22:51 +0800 Subject: [PATCH 1/2] add testcase & bug fixed --- .../alibaba/druid/pool/DruidDataSource.java | 30 ++++----- ...ruidDataSourceTest_getPoolingPeakTime.java | 1 + .../pool/DruidDataSourceTest_recycle2.java | 61 +++++++++++++++++-- .../com/alibaba/druid/bvt/pool/DumpTest.java | 13 ++++ 4 files changed, 83 insertions(+), 22 deletions(-) diff --git a/src/main/java/com/alibaba/druid/pool/DruidDataSource.java b/src/main/java/com/alibaba/druid/pool/DruidDataSource.java index 4299507fa5..4a6f3d94e2 100644 --- a/src/main/java/com/alibaba/druid/pool/DruidDataSource.java +++ b/src/main/java/com/alibaba/druid/pool/DruidDataSource.java @@ -444,13 +444,13 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat } private void createAndStartDestroyThread() { - DestroyConnectionThread thread = new DestroyConnectionThread("Druid-ConnectionPool-Destory"); - thread.start(); + destoryConnectionThread = new DestroyConnectionThread("Druid-ConnectionPool-Destory"); + destoryConnectionThread.start(); } protected void createAndStartCreatorThread() { - CreateConnectionThread thread = new CreateConnectionThread("Druid-ConnectionPool-Create"); - thread.start(); + createConnectionThread = new CreateConnectionThread("Druid-ConnectionPool-Create"); + createConnectionThread.start(); } private void loadFilterFromSystemProperty() throws SQLException { @@ -815,7 +815,6 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat } } - boolean neadDestory = false; lock.lockInterruptibly(); try { activeCount--; @@ -826,11 +825,6 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat } finally { lock.unlock(); } - - if (neadDestory) { - destroyCount.incrementAndGet(); - JdbcUtils.close(physicalConnection); - } } catch (Throwable e) { JdbcUtils.close(physicalConnection); @@ -894,7 +888,8 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat } connHolder.getStatementPool().getMap().clear(); - JdbcUtils.close(connHolder.getConnection()); + Connection physicalConnection = connHolder.getConnection(); + physicalConnection.close(); connections[i] = null; destroyCount.incrementAndGet(); } catch (Exception ex) { @@ -1348,12 +1343,9 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat } public String getProperties() { - if (this.connectProperties == null) { - return null; - } - - Properties properties = new Properties(this.connectProperties); - if (properties.contains("password")) { + Properties properties = new Properties(); + properties.putAll(connectProperties); + if (properties.containsKey("password")) { properties.put("password", "******"); } return properties.toString(); @@ -1538,7 +1530,7 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat int entryIndex = 0; try { for (Map.Entry entry : pool.getMap().entrySet()) { - if (entryIndex++ != 0) { + if (entryIndex != 0) { buf.append(","); } buf.append("\n\t\t{hitCount:"); @@ -1547,6 +1539,8 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat buf.append(entry.getKey().getSql()); buf.append("\""); buf.append("\t}"); + + entryIndex++; } } catch (ConcurrentModificationException e) { // skip .. diff --git a/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_getPoolingPeakTime.java b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_getPoolingPeakTime.java index 9b121e619d..0782940db8 100644 --- a/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_getPoolingPeakTime.java +++ b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_getPoolingPeakTime.java @@ -29,6 +29,7 @@ public class DruidDataSourceTest_getPoolingPeakTime extends TestCase { public void test_error() throws Exception { Assert.assertNull(dataSource.getPoolingPeakTime()); + Assert.assertNull(dataSource.getActivePeakTime()); Connection conn = dataSource.getConnection(); conn.close(); diff --git a/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_recycle2.java b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_recycle2.java index 03d11e07cd..f34346484c 100644 --- a/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_recycle2.java +++ b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_recycle2.java @@ -2,11 +2,14 @@ package com.alibaba.druid.bvt.pool; import java.sql.SQLException; import java.sql.Statement; - -import org.junit.Assert; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import junit.framework.TestCase; +import org.junit.Assert; + import com.alibaba.druid.filter.FilterAdapter; import com.alibaba.druid.filter.FilterChain; import com.alibaba.druid.pool.DruidDataSource; @@ -43,7 +46,7 @@ public class DruidDataSourceTest_recycle2 extends TestCase { Statement stmt = conn.createStatement(); stmt.execute("select 1"); - + Assert.assertEquals(0, dataSource.getPoolingCount()); Assert.assertEquals(1, dataSource.getActiveCount()); @@ -60,7 +63,7 @@ public class DruidDataSourceTest_recycle2 extends TestCase { Statement stmt = conn.createStatement(); stmt.execute("select 1"); - + Assert.assertEquals(0, dataSource.getPoolingCount()); Assert.assertEquals(1, dataSource.getActiveCount()); @@ -75,4 +78,54 @@ public class DruidDataSourceTest_recycle2 extends TestCase { Assert.assertEquals(0, dataSource.getPoolingCount()); Assert.assertEquals(0, dataSource.getActiveCount()); } + + public void test_recycle_error_interrupt() throws Exception { + final AtomicReference errorRef = new AtomicReference(); + + final CountDownLatch closeBeforeLatch = new CountDownLatch(1); + final CountDownLatch lockLatch = new CountDownLatch(1); + final CountDownLatch endLatch = new CountDownLatch(1); + + Thread thread = new Thread() { + + public void run() { + try { + DruidPooledConnection conn = dataSource.getConnection(); + conn.setAutoCommit(false); + conn.setReadOnly(false); + + Statement stmt = conn.createStatement(); + stmt.execute("select 1"); + + Assert.assertEquals(0, dataSource.getPoolingCount()); + Assert.assertEquals(1, dataSource.getActiveCount()); + + closeBeforeLatch.countDown(); + lockLatch.await(); + + conn.close(); + } catch (Exception e) { + errorRef.set(e); + } finally { + endLatch.countDown(); + } + } + }; + thread.start(); + + Assert.assertTrue(closeBeforeLatch.await(1, TimeUnit.SECONDS)); + + dataSource.getLock().lock(); + lockLatch.countDown(); + + Thread.sleep(10); + + thread.interrupt(); + + Assert.assertTrue(endLatch.await(1, TimeUnit.MINUTES)); + + Exception error = errorRef.get(); + Assert.assertNotNull(error); + Assert.assertTrue(error.getCause() instanceof InterruptedException); + } } diff --git a/src/test/java/com/alibaba/druid/bvt/pool/DumpTest.java b/src/test/java/com/alibaba/druid/bvt/pool/DumpTest.java index 0316b1cb76..90f94db0fa 100644 --- a/src/test/java/com/alibaba/druid/bvt/pool/DumpTest.java +++ b/src/test/java/com/alibaba/druid/bvt/pool/DumpTest.java @@ -60,6 +60,19 @@ public class DumpTest extends TestCase { } public void testToString() throws Exception { + { + String sql = "select ?"; + Connection conn = dataSource.getConnection(); + + PreparedStatement stmt = conn.prepareStatement(sql); + stmt.setString(1, "xxx"); + ResultSet rs = stmt.executeQuery(); + rs.next(); + rs.close(); + + conn.close(); + } + String sql = "select ?, ?"; Connection conn = dataSource.getConnection(); From 76eced38739593099045f6b6f7a939c32a0a1a29 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 25 Aug 2012 14:22:56 +0800 Subject: [PATCH 2/2] add testcase & bug fixed --- .../pool/DruidDataSourceTest_closeError.java | 41 ++++++++++ .../DruidDataSourceTest_getProperties.java | 34 ++++++++ .../bvt/pool/DruidDataSourceTest_maxWait.java | 79 +++++++++++++++++++ 3 files changed, 154 insertions(+) create mode 100644 src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_closeError.java create mode 100644 src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_getProperties.java create mode 100644 src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_maxWait.java diff --git a/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_closeError.java b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_closeError.java new file mode 100644 index 0000000000..c96e03d36b --- /dev/null +++ b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_closeError.java @@ -0,0 +1,41 @@ +package com.alibaba.druid.bvt.pool; + +import java.sql.SQLException; + +import junit.framework.TestCase; + +import com.alibaba.druid.filter.FilterAdapter; +import com.alibaba.druid.filter.FilterChain; +import com.alibaba.druid.pool.DruidDataSource; +import com.alibaba.druid.proxy.jdbc.ConnectionProxy; + +/** + * 这个场景测试initialSize > maxActive + * + * @author wenshao + */ +public class DruidDataSourceTest_closeError extends TestCase { + + private DruidDataSource dataSource; + + protected void setUp() throws Exception { + dataSource = new DruidDataSource(); + dataSource.setUrl("jdbc:mock:xxx"); + dataSource.setTestOnBorrow(false); + dataSource.setInitialSize(1); + dataSource.getProxyFilters().add(new FilterAdapter() { + + public void connection_close(FilterChain chain, ConnectionProxy connection) throws SQLException { + throw new SQLException(); + } + }); + dataSource.init(); + } + + protected void tearDown() throws Exception { + dataSource.close(); + } + + public void test_closeError() throws Exception { + } +} diff --git a/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_getProperties.java b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_getProperties.java new file mode 100644 index 0000000000..2318c7c63a --- /dev/null +++ b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_getProperties.java @@ -0,0 +1,34 @@ +package com.alibaba.druid.bvt.pool; + +import junit.framework.Assert; +import junit.framework.TestCase; + +import com.alibaba.druid.pool.DruidDataSource; + +/** + * 这个场景测试initialSize > maxActive + * + * @author wenshao + */ +public class DruidDataSourceTest_getProperties extends TestCase { + + private DruidDataSource dataSource; + + protected void setUp() throws Exception { + dataSource = new DruidDataSource(); + dataSource.setUrl("jdbc:mock:xxx"); + dataSource.setTestOnBorrow(false); + dataSource.getConnectProperties().put("user", "jobs"); + dataSource.getConnectProperties().put("password", "xxx"); + dataSource.init(); + } + + protected void tearDown() throws Exception { + dataSource.close(); + } + + public void test_error() throws Exception { + Assert.assertEquals(-1, dataSource.getProperties().indexOf("xxx")); + Assert.assertEquals(true, dataSource.getProperties().indexOf("******") != -1); + } +} diff --git a/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_maxWait.java b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_maxWait.java new file mode 100644 index 0000000000..8f4631557b --- /dev/null +++ b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_maxWait.java @@ -0,0 +1,79 @@ +package com.alibaba.druid.bvt.pool; + +import java.sql.Connection; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicInteger; + +import junit.framework.Assert; +import junit.framework.TestCase; + +import com.alibaba.druid.pool.DruidDataSource; + +/** + * 这个场景测试initialSize > maxActive + * + * @author wenshao + */ +public class DruidDataSourceTest_maxWait extends TestCase { + + private DruidDataSource dataSource; + + protected void setUp() throws Exception { + dataSource = new DruidDataSource(); + dataSource.setUrl("jdbc:mock:xxx"); + dataSource.setTestOnBorrow(false); + dataSource.setMaxActive(1); + dataSource.setMaxWait(30); + dataSource.setInitialSize(1); + dataSource.init(); + } + + protected void tearDown() throws Exception { + dataSource.close(); + } + + public void test_maxWait() throws Exception { + Connection conn = dataSource.getConnection(); + + final AtomicInteger errorCount = new AtomicInteger(); + final CountDownLatch endLatch = new CountDownLatch(2); + + Thread t1 = new Thread("t-1") { + + public void run() { + try { + dataSource.getConnection(); + } catch (Exception e) { + errorCount.incrementAndGet(); + } finally { + endLatch.countDown(); + } + } + }; + + Thread t2 = new Thread("t-2") { + + public void run() { + try { + dataSource.getConnection(); + } catch (Exception e) { + errorCount.incrementAndGet(); + } finally { + endLatch.countDown(); + } + } + }; + + t1.start(); + t2.start(); + + Thread.sleep(10); + t1.interrupt(); + + endLatch.await(); + + Assert.assertEquals(2, errorCount.get()); + + conn.close(); + } +}