You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2019/07/29 23:01:15 UTC
[commons-dbcp] branch release updated: [DBCP-549] Make
org.apache.commons.dbcp2.AbandonedTrace.removeTrace(AbandonedTrace)
null-safe.
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch release
in repository https://gitbox.apache.org/repos/asf/commons-dbcp.git
The following commit(s) were added to refs/heads/release by this push:
new 6caf93b [DBCP-549] Make org.apache.commons.dbcp2.AbandonedTrace.removeTrace(AbandonedTrace) null-safe.
6caf93b is described below
commit 6caf93beaf7e4207ef72c9ca17be75a1244d3470
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Mon Jul 29 19:01:07 2019 -0400
[DBCP-549] Make
org.apache.commons.dbcp2.AbandonedTrace.removeTrace(AbandonedTrace)
null-safe.
- Also refactor pattern into new protected method
org.apache.commons.dbcp2.AbandonedTrace.removeTrace(Object,
AbandonedTrace).
- Remove trailing white spaces on all lines
- Use final.
---
src/changes/changes.xml | 3 +++
.../org/apache/commons/dbcp2/AbandonedTrace.java | 25 ++++++++++++++----
.../org/apache/commons/dbcp2/BasicDataSource.java | 6 ++---
.../apache/commons/dbcp2/DelegatingResultSet.java | 4 +--
.../apache/commons/dbcp2/DelegatingStatement.java | 10 ++++----
.../commons/dbcp2/PoolableCallableStatement.java | 8 ++----
.../commons/dbcp2/PoolableConnectionFactory.java | 30 +++++++++++-----------
.../commons/dbcp2/PoolablePreparedStatement.java | 8 ++----
8 files changed, 52 insertions(+), 42 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index ce7977e..394baa8 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -79,6 +79,9 @@ The <action> type attribute can be add,update,fix,remove.
<action dev="ggregory" type="update" issue="DBCP-548" due-to="Gary Gregory">
Update Apache Commons Pool from 2.6.1 to 2.7.0.
</action>
+ <action dev="ggregory" type="update" issue="DBCP-549" due-to="Gary Gregory">
+ Make org.apache.commons.dbcp2.AbandonedTrace.removeTrace(AbandonedTrace) null-safe.
+ </action>
</release>
<release version="2.6.0" date="2019-02-14" description="This is a minor release, including bug fixes and enhancements.">
<action dev="chtompki" type="add" issue="DBCP-534" due-to="Peter Wicks">
diff --git a/src/main/java/org/apache/commons/dbcp2/AbandonedTrace.java b/src/main/java/org/apache/commons/dbcp2/AbandonedTrace.java
index 659fe76..7b8a79f 100644
--- a/src/main/java/org/apache/commons/dbcp2/AbandonedTrace.java
+++ b/src/main/java/org/apache/commons/dbcp2/AbandonedTrace.java
@@ -100,13 +100,15 @@ public class AbandonedTrace implements TrackedUse {
* Adds an object to the list of objects being traced.
*
* @param trace
- * AbandonedTrace object to add.
+ * AbandonedTrace object to add, null is ignored.
*/
protected void addTrace(final AbandonedTrace trace) {
- synchronized (this.traceList) {
- this.traceList.add(new WeakReference<>(trace));
+ if (trace != null) {
+ synchronized (this.traceList) {
+ this.traceList.add(new WeakReference<>(trace));
+ }
+ setLastUsed();
}
- setLastUsed();
}
/**
@@ -155,7 +157,7 @@ public class AbandonedTrace implements TrackedUse {
final Iterator<WeakReference<AbandonedTrace>> iter = traceList.iterator();
while (iter.hasNext()) {
final AbandonedTrace traceInList = iter.next().get();
- if (trace.equals(traceInList)) {
+ if (trace != null && trace.equals(traceInList)) {
iter.remove();
break;
} else if (traceInList == null) {
@@ -165,4 +167,17 @@ public class AbandonedTrace implements TrackedUse {
}
}
}
+
+ /**
+ * Removes a child object the given source is tracing.
+ *
+ * @param source The object from which to remove the given {@code trace}, may be null.
+ * @param trace AbandonedTrace to remove.
+ * @since 2.7.0
+ */
+ protected void removeTrace(final Object source, final AbandonedTrace trace) {
+ if (source instanceof AbandonedTrace) {
+ ((AbandonedTrace) source).removeTrace(trace);
+ }
+ }
}
diff --git a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java
index 25d5981..9b66dc2 100644
--- a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java
+++ b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java
@@ -1528,14 +1528,14 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
/**
* Logs the given throwable.
- *
+ *
* @param throwable the throwable.
* @since 2.7.0
*/
- protected void log(Throwable throwable) {
+ protected void log(final Throwable throwable) {
if (logWriter != null) {
throwable.printStackTrace(logWriter);
- }
+ }
}
@Override
diff --git a/src/main/java/org/apache/commons/dbcp2/DelegatingResultSet.java b/src/main/java/org/apache/commons/dbcp2/DelegatingResultSet.java
index 3d1483c..968975e 100644
--- a/src/main/java/org/apache/commons/dbcp2/DelegatingResultSet.java
+++ b/src/main/java/org/apache/commons/dbcp2/DelegatingResultSet.java
@@ -186,11 +186,11 @@ public final class DelegatingResultSet extends AbandonedTrace implements ResultS
public void close() throws SQLException {
try {
if (statement != null) {
- ((AbandonedTrace) statement).removeTrace(this);
+ removeTrace(statement, this);
statement = null;
}
if (connection != null) {
- ((AbandonedTrace) connection).removeTrace(this);
+ removeTrace(connection, this);
connection = null;
}
resultSet.close();
diff --git a/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java b/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java
index 8a1725f..b636704 100644
--- a/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java
+++ b/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java
@@ -136,11 +136,11 @@ public class DelegatingStatement extends AbandonedTrace implements Statement {
// ResultSet's when it is closed.
// FIXME The PreparedStatement we're wrapping should handle this for us.
// See bug 17301 for what could happen when ResultSets are closed twice.
- final List<AbandonedTrace> resultSets = getTrace();
- if (resultSets != null) {
- final ResultSet[] set = resultSets.toArray(new ResultSet[resultSets.size()]);
- for (final ResultSet element : set) {
- element.close();
+ final List<AbandonedTrace> resultSetList = getTrace();
+ if (resultSetList != null) {
+ final ResultSet[] resultSets = resultSetList.toArray(new ResultSet[resultSetList.size()]);
+ for (final ResultSet resultSet : resultSets) {
+ resultSet.close();
}
clearTrace();
}
diff --git a/src/main/java/org/apache/commons/dbcp2/PoolableCallableStatement.java b/src/main/java/org/apache/commons/dbcp2/PoolableCallableStatement.java
index 969a7e1..3d1709e 100644
--- a/src/main/java/org/apache/commons/dbcp2/PoolableCallableStatement.java
+++ b/src/main/java/org/apache/commons/dbcp2/PoolableCallableStatement.java
@@ -67,9 +67,7 @@ public class PoolableCallableStatement extends DelegatingCallableStatement {
// Remove from trace now because this statement will be
// added by the activate method.
- if (getConnectionInternal() != null) {
- getConnectionInternal().removeTrace(this);
- }
+ removeTrace(getConnectionInternal(), this);
}
/**
@@ -115,9 +113,7 @@ public class PoolableCallableStatement extends DelegatingCallableStatement {
@Override
public void passivate() throws SQLException {
setClosedInternal(true);
- if (getConnectionInternal() != null) {
- getConnectionInternal().removeTrace(this);
- }
+ removeTrace(getConnectionInternal(), this);
// The JDBC spec requires that a statement close any open
// ResultSet's when it is closed.
diff --git a/src/main/java/org/apache/commons/dbcp2/PoolableConnectionFactory.java b/src/main/java/org/apache/commons/dbcp2/PoolableConnectionFactory.java
index cc91ec4..fd3e176 100644
--- a/src/main/java/org/apache/commons/dbcp2/PoolableConnectionFactory.java
+++ b/src/main/java/org/apache/commons/dbcp2/PoolableConnectionFactory.java
@@ -312,7 +312,7 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
}
}
}
-
+
/**
* @return Whether to auto-commit on return.
* @since 2.6.0
@@ -329,7 +329,7 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
public boolean isEnableAutoCommitOnReturn() {
return autoCommitOnReturn;
}
-
+
/**
* True means that validation will fail immediately for connections that have previously thrown SQLExceptions with
* SQL_STATE indicating fatal disconnection errors.
@@ -342,14 +342,14 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
public boolean isFastFailValidation() {
return fastFailValidation;
}
-
+
/**
* @return Whether to rollback on return.
*/
public boolean isRollbackOnReturn() {
return rollbackOnReturn;
}
-
+
@Override
public PooledObject<PoolableConnection> makeObject() throws Exception {
Connection conn = connectionFactory.createConnection();
@@ -410,7 +410,7 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
return new DefaultPooledObject<>(pc);
}
-
+
@Override
public void passivateObject(final PooledObject<PoolableConnection> p) throws Exception {
@@ -440,15 +440,15 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
conn.passivate();
}
-
+
public void setAutoCommitOnReturn(final boolean autoCommitOnReturn) {
this.autoCommitOnReturn = autoCommitOnReturn;
}
-
+
public void setCacheState(final boolean cacheState) {
this.cacheState = cacheState;
}
-
+
/**
* Sets the SQL statements I use to initialize newly created {@link Connection}s. Using {@code null} turns off
* connection initialization.
@@ -459,7 +459,7 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
public void setConnectionInitSql(final Collection<String> connectionInitSqls) {
this.connectionInitSqls = connectionInitSqls;
}
-
+
/**
* Sets the default "auto commit" setting for borrowed {@link Connection}s
*
@@ -469,7 +469,7 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
public void setDefaultAutoCommit(final Boolean defaultAutoCommit) {
this.defaultAutoCommit = defaultAutoCommit;
}
-
+
/**
* Sets the default "catalog" setting for borrowed {@link Connection}s
*
@@ -479,7 +479,7 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
public void setDefaultCatalog(final String defaultCatalog) {
this.defaultCatalog = defaultCatalog;
}
-
+
public void setDefaultQueryTimeout(final Integer defaultQueryTimeoutSeconds) {
this.defaultQueryTimeoutSeconds = defaultQueryTimeoutSeconds;
}
@@ -492,7 +492,7 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
public void setDefaultReadOnly(final Boolean defaultReadOnly) {
this.defaultReadOnly = defaultReadOnly;
}
-
+
/**
* Sets the default "schema" setting for borrowed {@link Connection}s
*
@@ -503,7 +503,7 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
public void setDefaultSchema(final String defaultSchema) {
this.defaultSchema = defaultSchema;
}
-
+
/**
* Sets the default "Transaction Isolation" setting for borrowed {@link Connection}s
*
@@ -513,7 +513,7 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
public void setDefaultTransactionIsolation(final int defaultTransactionIsolation) {
this.defaultTransactionIsolation = defaultTransactionIsolation;
}
-
+
/**
* @param disconnectionSqlCodes
* The disconnection SQL codes.
@@ -523,7 +523,7 @@ public class PoolableConnectionFactory implements PooledObjectFactory<PoolableCo
public void setDisconnectionSqlCodes(final Collection<String> disconnectionSqlCodes) {
this.disconnectionSqlCodes = disconnectionSqlCodes;
}
-
+
/**
* @param autoCommitOnReturn Whether to auto-commit on return.
* @deprecated Use {@link #setAutoCommitOnReturn(boolean)}.
diff --git a/src/main/java/org/apache/commons/dbcp2/PoolablePreparedStatement.java b/src/main/java/org/apache/commons/dbcp2/PoolablePreparedStatement.java
index 15d933e..62825d7 100644
--- a/src/main/java/org/apache/commons/dbcp2/PoolablePreparedStatement.java
+++ b/src/main/java/org/apache/commons/dbcp2/PoolablePreparedStatement.java
@@ -69,9 +69,7 @@ public class PoolablePreparedStatement<K> extends DelegatingPreparedStatement {
// Remove from trace now because this statement will be
// added by the activate method.
- if (getConnectionInternal() != null) {
- getConnectionInternal().removeTrace(this);
- }
+ removeTrace(getConnectionInternal(), this);
}
/**
@@ -128,9 +126,7 @@ public class PoolablePreparedStatement<K> extends DelegatingPreparedStatement {
clearBatch();
}
setClosedInternal(true);
- if (getConnectionInternal() != null) {
- getConnectionInternal().removeTrace(this);
- }
+ removeTrace(getConnectionInternal(), this);
// The JDBC spec requires that a statement closes any open
// ResultSet's when it is closed.