From a92d21cfab326fc7f4152646fb14c78bab054351 Mon Sep 17 00:00:00 2001 From: wenshao Date: Thu, 8 Dec 2011 09:58:32 +0800 Subject: [PATCH] enhance management --- .../druid/mock/MockPreparedStatement.java | 34 +--- .../com/alibaba/druid/mock/MockStatement.java | 163 ++++++------------ .../druid/pool/DruidAbstractDataSource.java | 11 +- .../alibaba/druid/pool/DruidDataSource.java | 27 ++- .../pool/DruidDataSourceC3P0Adapter.java | 4 + .../stat/DruidDataSourceStatManager.java | 2 + .../com/alibaba/druid/util/JdbcUtils.java | 9 + .../com/alibaba/druid/Bug_for_alibank.java | 15 ++ .../bvt/pool/Bug_for_dupCloseStmtError.java | 47 +++++ .../druid/bvt/pool/TestConcurrent.java | 10 +- .../bvt/pool/basic/TestDataSourceBasic2.java | 2 +- .../druid/bvt/pool/ha/HADataSourceTest.java | 9 + 12 files changed, 178 insertions(+), 155 deletions(-) create mode 100644 src/test/java/com/alibaba/druid/Bug_for_alibank.java create mode 100644 src/test/java/com/alibaba/druid/bvt/pool/Bug_for_dupCloseStmtError.java diff --git a/src/main/java/com/alibaba/druid/mock/MockPreparedStatement.java b/src/main/java/com/alibaba/druid/mock/MockPreparedStatement.java index a15855700b..38cec850c0 100644 --- a/src/main/java/com/alibaba/druid/mock/MockPreparedStatement.java +++ b/src/main/java/com/alibaba/druid/mock/MockPreparedStatement.java @@ -60,9 +60,7 @@ public class MockPreparedStatement extends MockStatement implements PreparedStat @Override public ResultSet executeQuery() throws SQLException { - if (closed) { - throw new SQLException(); - } + checkOpen(); if (mockConnection != null && mockConnection.getDriver() != null) { return mockConnection.getDriver().createResultSet(this); @@ -77,9 +75,7 @@ public class MockPreparedStatement extends MockStatement implements PreparedStat @Override public int executeUpdate() throws SQLException { - if (closed) { - throw new SQLException(); - } + checkOpen(); return 0; } @@ -171,13 +167,7 @@ public class MockPreparedStatement extends MockStatement implements PreparedStat @Override public void clearParameters() throws SQLException { - if (closed) { - throw new SQLException(); - } - - if (this.mockConnection != null && this.mockConnection.isClosed()) { - throw new SQLException("No operations allowed after connection closed."); - } + checkOpen(); parameters.clear(); } @@ -194,17 +184,14 @@ public class MockPreparedStatement extends MockStatement implements PreparedStat @Override public boolean execute() throws SQLException { - if (closed) { - throw new SQLException(); - } + checkOpen(); + return false; } @Override public void addBatch() throws SQLException { - if (closed) { - throw new SQLException(); - } + checkOpen(); } @Override @@ -234,9 +221,7 @@ public class MockPreparedStatement extends MockStatement implements PreparedStat @Override public ResultSetMetaData getMetaData() throws SQLException { - if (closed) { - throw new SQLException(); - } + checkOpen(); return resultSetMetaData; } @@ -268,10 +253,7 @@ public class MockPreparedStatement extends MockStatement implements PreparedStat @Override public ParameterMetaData getParameterMetaData() throws SQLException { - if (closed) { - throw new SQLException(); - } - + checkOpen(); return metadata; } diff --git a/src/main/java/com/alibaba/druid/mock/MockStatement.java b/src/main/java/com/alibaba/druid/mock/MockStatement.java index 985e717fa2..7097602b60 100644 --- a/src/main/java/com/alibaba/druid/mock/MockStatement.java +++ b/src/main/java/com/alibaba/druid/mock/MockStatement.java @@ -52,11 +52,19 @@ public class MockStatement implements Statement { mockConnection = (MockConnection) connection; } } + + protected void checkOpen() throws SQLException, MockConnectionClosedException { + if (closed) { + throw new SQLException(); + } + + if (this.mockConnection != null && this.mockConnection.isClosed()) { + throw new MockConnectionClosedException(); + } + } public int getResultSetType() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return resultSetType; } @@ -107,9 +115,7 @@ public class MockStatement implements Statement { @Override public ResultSet executeQuery(String sql) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); if (mockConnection != null && mockConnection.getDriver() != null) { return mockConnection.getDriver().executeQuery(this, sql); @@ -120,9 +126,7 @@ public class MockStatement implements Statement { @Override public int executeUpdate(String sql) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return 0; } @@ -134,46 +138,35 @@ public class MockStatement implements Statement { @Override public int getMaxFieldSize() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return maxFieldSize; } @Override public void setMaxFieldSize(int max) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); this.maxFieldSize = max; } @Override public int getMaxRows() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return maxRows; } @Override public void setMaxRows(int max) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); this.maxRows = max; } @Override public void setEscapeProcessing(boolean enable) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } - + checkOpen(); this.escapeProcessing = enable; } @@ -183,41 +176,31 @@ public class MockStatement implements Statement { @Override public int getQueryTimeout() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return queryTimeout; } @Override public void setQueryTimeout(int seconds) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); this.queryTimeout = seconds; } @Override public void cancel() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); } @Override public SQLWarning getWarnings() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return warnings; } @Override public void clearWarnings() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); warnings = null; } @@ -228,9 +211,7 @@ public class MockStatement implements Statement { @Override public void setCursorName(String name) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); cursorName = name; } @@ -241,9 +222,7 @@ public class MockStatement implements Statement { @Override public boolean execute(String sql) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); if (ERROR_SQL.equals(sql)) { throw new SQLException(); @@ -254,18 +233,14 @@ public class MockStatement implements Statement { @Override public ResultSet getResultSet() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return new MockResultSet(this); } @Override public int getUpdateCount() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return updateCount; } @@ -276,153 +251,115 @@ public class MockStatement implements Statement { @Override public boolean getMoreResults() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return false; } @Override public void setFetchDirection(int direction) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); this.fetchDirection = direction; } @Override public int getFetchDirection() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return fetchDirection; } @Override public void setFetchSize(int rows) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); this.fetchSize = rows; } @Override public int getFetchSize() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return fetchSize; } @Override public int getResultSetConcurrency() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return resultSetConcurrency; } @Override public void addBatch(String sql) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); } @Override public void clearBatch() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); } @Override public int[] executeBatch() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return new int[0]; } @Override public Connection getConnection() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return connection; } @Override public boolean getMoreResults(int current) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return false; } @Override public ResultSet getGeneratedKeys() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return new MockResultSet(this); } @Override public int executeUpdate(String sql, int autoGeneratedKeys) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return 0; } @Override public int executeUpdate(String sql, int[] columnIndexes) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return 0; } @Override public int executeUpdate(String sql, String[] columnNames) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return 0; } @Override public boolean execute(String sql, int autoGeneratedKeys) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return false; } @Override public boolean execute(String sql, int[] columnIndexes) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return false; } @Override public boolean execute(String sql, String[] columnNames) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return false; } @Override public int getResultSetHoldability() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return resultSetHoldability; } @@ -437,16 +374,12 @@ public class MockStatement implements Statement { @Override public void setPoolable(boolean poolable) throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); } @Override public boolean isPoolable() throws SQLException { - if (closed) { - throw new SQLException("stmt closed."); - } + checkOpen(); return false; } diff --git a/src/main/java/com/alibaba/druid/pool/DruidAbstractDataSource.java b/src/main/java/com/alibaba/druid/pool/DruidAbstractDataSource.java index 8857091c1b..d558d55814 100644 --- a/src/main/java/com/alibaba/druid/pool/DruidAbstractDataSource.java +++ b/src/main/java/com/alibaba/druid/pool/DruidAbstractDataSource.java @@ -394,11 +394,11 @@ public abstract class DruidAbstractDataSource extends WrapperAdapter implements } public int getMaxOpenPreparedStatements() { - return maxOpenPreparedStatements; + return maxPoolPreparedStatementPerConnectionSize; } public void setMaxOpenPreparedStatements(int maxOpenPreparedStatements) { - this.maxOpenPreparedStatements = maxOpenPreparedStatements; + this.maxPoolPreparedStatementPerConnectionSize = maxOpenPreparedStatements; } public boolean isLogAbandoned() { @@ -963,12 +963,7 @@ public abstract class DruidAbstractDataSource extends WrapperAdapter implements public List getActiveConnectionStackTrace() { List list = new ArrayList(); for (ActiveConnectionTraceInfo traceInfo : this.activeConnections.values()) { - StringBuilder buf = new StringBuilder(); - for (StackTraceElement item : traceInfo.getStackTrace()) { - buf.append(item.toString()); - buf.append("\n"); - } - list.add(buf.toString()); + list.add(JdbcUtils.toString(traceInfo.getStackTrace())); } return list; diff --git a/src/main/java/com/alibaba/druid/pool/DruidDataSource.java b/src/main/java/com/alibaba/druid/pool/DruidDataSource.java index 03d15d151d..a28ba2a5aa 100644 --- a/src/main/java/com/alibaba/druid/pool/DruidDataSource.java +++ b/src/main/java/com/alibaba/druid/pool/DruidDataSource.java @@ -30,6 +30,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.ReentrantLock; +import javax.management.ObjectName; import javax.naming.NamingException; import javax.naming.Reference; import javax.naming.Referenceable; @@ -105,9 +106,25 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat private boolean resetStatEnable = true; + private String initStackTrace; + + private ObjectName objectName; + public DruidDataSource(){ } + public String getInitStackTrace() { + return initStackTrace; + } + + public ObjectName getObjectName() { + return objectName; + } + + public void setObjectName(ObjectName objectName) { + this.objectName = objectName; + } + public boolean isResetStatEnable() { return resetStatEnable; } @@ -210,6 +227,8 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat return; } + initStackTrace = JdbcUtils.toString(Thread.currentThread().getStackTrace()); + this.id = DruidDriver.createDataSourceId(); enable = true; @@ -329,7 +348,7 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat lock.unlock(); } } - + @Override public Connection getConnection() throws SQLException { return getConnection(maxWait); @@ -538,7 +557,7 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat if ((!isAutoCommit) && (!isReadOnly)) { pooledConnection.rollback(); } - + // reset holder, restore default settings, clear warnings holder.reset(); @@ -922,7 +941,7 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat long currentMillis = System.currentTimeMillis(); List abondonedList = new ArrayList(); - + for (; iter.hasNext();) { Map.Entry entry = iter.next(); ActiveConnectionTraceInfo activeInfo = entry.getValue(); @@ -950,7 +969,7 @@ public class DruidDataSource extends DruidAbstractDataSource implements DruidDat } } } - + // multi-check dup close for (PoolableConnection conn : abondonedList) { activeConnections.remove(conn); diff --git a/src/main/java/com/alibaba/druid/pool/DruidDataSourceC3P0Adapter.java b/src/main/java/com/alibaba/druid/pool/DruidDataSourceC3P0Adapter.java index ab37811304..c788b1360b 100644 --- a/src/main/java/com/alibaba/druid/pool/DruidDataSourceC3P0Adapter.java +++ b/src/main/java/com/alibaba/druid/pool/DruidDataSourceC3P0Adapter.java @@ -264,6 +264,8 @@ public class DruidDataSourceC3P0Adapter implements DataSource, DruidDataSourceC3 } public void setMaxStatementsPerConnection(int maxStatementsPerConnection) { + dataSource.setPoolPreparedStatements(maxStatementsPerConnection > 0); + dataSource.setMaxPoolPreparedStatementPerConnectionSize(maxStatementsPerConnection); } @@ -272,6 +274,8 @@ public class DruidDataSourceC3P0Adapter implements DataSource, DruidDataSourceC3 } public void setMaxStatements(int maxStatements) { + dataSource.setPoolPreparedStatements(maxStatements > 0); + dataSource.setMaxOpenPreparedStatements(maxStatements); } diff --git a/src/main/java/com/alibaba/druid/stat/DruidDataSourceStatManager.java b/src/main/java/com/alibaba/druid/stat/DruidDataSourceStatManager.java index 24678e968f..1893477439 100644 --- a/src/main/java/com/alibaba/druid/stat/DruidDataSourceStatManager.java +++ b/src/main/java/com/alibaba/druid/stat/DruidDataSourceStatManager.java @@ -88,12 +88,14 @@ public class DruidDataSourceStatManager implements DruidDataSourceStatManagerMBe } dataSources.put(dataSource, objectName); + dataSource.setObjectName(objectName); } public synchronized static void remove(DruidDataSource dataSource) { ObjectName objectName = dataSources.remove(dataSource); if (objectName == null) { + LOG.error("unregister mbean failed."); return; } diff --git a/src/main/java/com/alibaba/druid/util/JdbcUtils.java b/src/main/java/com/alibaba/druid/util/JdbcUtils.java index 6c6ad3c3c7..8106965653 100644 --- a/src/main/java/com/alibaba/druid/util/JdbcUtils.java +++ b/src/main/java/com/alibaba/druid/util/JdbcUtils.java @@ -516,4 +516,13 @@ public final class JdbcUtils { throw new SQLException(e.getMessage(), e); } } + + public static String toString(StackTraceElement[] stackTrace) { + StringBuilder buf = new StringBuilder(); + for (StackTraceElement item : stackTrace) { + buf.append(item.toString()); + buf.append("\n"); + } + return buf.toString(); + } } diff --git a/src/test/java/com/alibaba/druid/Bug_for_alibank.java b/src/test/java/com/alibaba/druid/Bug_for_alibank.java new file mode 100644 index 0000000000..020ff1d78f --- /dev/null +++ b/src/test/java/com/alibaba/druid/Bug_for_alibank.java @@ -0,0 +1,15 @@ +package com.alibaba.druid; + +import junit.framework.TestCase; + +import com.alibaba.fastjson.JSON; + + +public class Bug_for_alibank extends TestCase { + public void test_bug() throws Exception { + String jsonStrz = "{addContact:[{\"address\":\"=\\\\\\\\\\\'\'\\");|]*{%0d%0a<%00\"}]}"; + System.out.println(jsonStrz); + Object o = JSON.parseObject(jsonStrz.replaceAll("\\\\", "")); + System.out.println(JSON.toJSONString(o)); + } +} diff --git a/src/test/java/com/alibaba/druid/bvt/pool/Bug_for_dupCloseStmtError.java b/src/test/java/com/alibaba/druid/bvt/pool/Bug_for_dupCloseStmtError.java new file mode 100644 index 0000000000..139938ef7e --- /dev/null +++ b/src/test/java/com/alibaba/druid/bvt/pool/Bug_for_dupCloseStmtError.java @@ -0,0 +1,47 @@ +package com.alibaba.druid.bvt.pool; + +import java.sql.Connection; +import java.sql.PreparedStatement; + +import junit.framework.TestCase; + +import com.alibaba.druid.mock.MockConnection; +import com.alibaba.druid.pool.DruidDataSource; +import com.alibaba.druid.stat.DruidDataSourceStatManager; + +public class Bug_for_dupCloseStmtError extends TestCase { + + protected DruidDataSource dataSource; + + protected void setUp() throws Exception { + dataSource = new DruidDataSource(); + dataSource.setUrl("jdbc:mock:"); + dataSource.setPoolPreparedStatements(false); + } + + protected void tearDown() throws Exception { + dataSource.close(); + for (DruidDataSource dataSource : DruidDataSourceStatManager.getDruidDataSourceInstances()) { + dataSource.close(); + } + } + + public void test_0() throws Exception { + { + Connection conn = dataSource.getConnection(); + PreparedStatement stmt = conn.prepareStatement("select 1"); + stmt.setString(1, "xx"); + + MockConnection mockConn = conn.unwrap(MockConnection.class); + mockConn.close(); + conn.close(); + stmt.close(); + } + { + Connection conn = dataSource.getConnection(); + PreparedStatement stmt = conn.prepareStatement("select 1"); + stmt.close(); + conn.close(); + } + } +} diff --git a/src/test/java/com/alibaba/druid/bvt/pool/TestConcurrent.java b/src/test/java/com/alibaba/druid/bvt/pool/TestConcurrent.java index ee5c0b4ff0..5897a4278d 100644 --- a/src/test/java/com/alibaba/druid/bvt/pool/TestConcurrent.java +++ b/src/test/java/com/alibaba/druid/bvt/pool/TestConcurrent.java @@ -44,7 +44,15 @@ public class TestConcurrent extends TestCase { protected void tearDown() throws Exception { dataSource.close(); - Assert.assertEquals(0, DruidDataSourceStatManager.getInstance().getDataSourceList().size()); + + int size = DruidDataSourceStatManager.getInstance().getDataSourceList().size(); + if (size > 0) { + for (DruidDataSource dataSource : DruidDataSourceStatManager.getDruidDataSourceInstances()) { + dataSource.close(); + System.out.println("unclosed datasource : " + dataSource.getObjectName()); + } + Assert.fail("size : " + size); + } } public void test_0() throws Exception { diff --git a/src/test/java/com/alibaba/druid/bvt/pool/basic/TestDataSourceBasic2.java b/src/test/java/com/alibaba/druid/bvt/pool/basic/TestDataSourceBasic2.java index 8e9d3c2a81..c7edc46719 100644 --- a/src/test/java/com/alibaba/druid/bvt/pool/basic/TestDataSourceBasic2.java +++ b/src/test/java/com/alibaba/druid/bvt/pool/basic/TestDataSourceBasic2.java @@ -42,7 +42,7 @@ public class TestDataSourceBasic2 extends TestCase { Assert.assertEquals(1, dataSource.getConnectionInitSqls().size()); Assert.assertEquals(30 * 1000, dataSource.getTimeBetweenConnectErrorMillis()); - Assert.assertEquals(-1, dataSource.getMaxOpenPreparedStatements()); + Assert.assertEquals(234, dataSource.getMaxOpenPreparedStatements()); Assert.assertEquals(300, dataSource.getRemoveAbandonedTimeout()); dataSource.setRemoveAbandonedTimeout(400); Assert.assertEquals(400, dataSource.getRemoveAbandonedTimeout()); diff --git a/src/test/java/com/alibaba/druid/bvt/pool/ha/HADataSourceTest.java b/src/test/java/com/alibaba/druid/bvt/pool/ha/HADataSourceTest.java index 7e9d22f6f0..2ee245c187 100644 --- a/src/test/java/com/alibaba/druid/bvt/pool/ha/HADataSourceTest.java +++ b/src/test/java/com/alibaba/druid/bvt/pool/ha/HADataSourceTest.java @@ -11,6 +11,7 @@ import com.alibaba.druid.mock.MockStatement; import com.alibaba.druid.pool.DruidDataSource; import com.alibaba.druid.pool.ha.HADataSource; import com.alibaba.druid.pool.ha.MultiDataSourceStatement; +import com.alibaba.druid.stat.DruidDataSourceStatManager; public class HADataSourceTest extends TestCase { @@ -20,6 +21,8 @@ public class HADataSourceTest extends TestCase { private HADataSource dataSourceHA; protected void setUp() throws Exception { + Assert.assertEquals(0, DruidDataSourceStatManager.getInstance().getDataSourceList().size()); + dataSourceA = new DruidDataSource(); dataSourceA.setUrl("jdbc:mock:x1"); dataSourceA.setFilters("trace"); @@ -35,6 +38,12 @@ public class HADataSourceTest extends TestCase { protected void tearDown() throws Exception { dataSourceHA.close(); + + for (DruidDataSource dataSource : DruidDataSourceStatManager.getDruidDataSourceInstances()) { + dataSource.close(); + } + + Assert.assertEquals(0, DruidDataSourceStatManager.getInstance().getDataSourceList().size()); } public void test_createStatement_0() throws Exception {