You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ma...@apache.org on 2017/11/10 17:39:38 UTC
commons-pool git commit: POOL-335: Make abandoned object logging
verbosity configurable
Repository: commons-pool
Updated Branches:
refs/heads/master 26f072ec6 -> 3956eedbf
POOL-335: Make abandoned object logging verbosity configurable
This reverts the default behavior introduced by POOL-320 where abandoned
object tracking would print less information by default when a
SecurityManager was usable. This adds a new configuration option to
AbandonedConfig to change that default behavior to use either a full
stack trace (default) or an abbreviated one with only class names.
Project: http://git-wip-us.apache.org/repos/asf/commons-pool/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-pool/commit/3956eedb
Tree: http://git-wip-us.apache.org/repos/asf/commons-pool/tree/3956eedb
Diff: http://git-wip-us.apache.org/repos/asf/commons-pool/diff/3956eedb
Branch: refs/heads/master
Commit: 3956eedbfcd8f57263892bb6667d21a839f67990
Parents: 26f072e
Author: Matt Sicker <bo...@gmail.com>
Authored: Fri Nov 10 11:37:38 2017 -0600
Committer: Matt Sicker <bo...@gmail.com>
Committed: Fri Nov 10 11:37:38 2017 -0600
----------------------------------------------------------------------
src/changes/changes.xml | 4 ++
.../org/apache/commons/pool2/PooledObject.java | 12 +++++
.../commons/pool2/impl/AbandonedConfig.java | 41 ++++++++++++++++-
.../apache/commons/pool2/impl/CallStack.java | 2 +-
.../commons/pool2/impl/CallStackUtils.java | 24 +++++++++-
.../commons/pool2/impl/DefaultPooledObject.java | 26 +++++++++--
.../commons/pool2/impl/GenericObjectPool.java | 23 ++++++----
.../commons/pool2/impl/NoOpCallStack.java | 48 ++++++++++++++++++++
.../commons/pool2/impl/ThrowableCallStack.java | 6 +--
.../commons/pool2/impl/NoOpCallStackTest.java | 34 ++++++++++++++
10 files changed, 199 insertions(+), 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3956eedb/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 8df6291..59f8f9e 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -59,6 +59,10 @@ The <action> type attribute can be add,update,fix,remove.
<action dev="ggregory" issue="POOL-334" type="update">
org.apache.commons.pool2.impl.ThrowableCallStack.Snapshot is missing serialVersionUID.
</action>
+ <action dev="mattsicker" issue="POOL-335" type="add">
+ Make abandoned logging stack trace requirements configurable. This also reverts
+ the default behavior introduced by POOL-320.
+ </action>
</release>
<release version="2.4.3" date="2017-10-24" description="This is a patch release, including bug fixes only.">
<action dev="ggregory" issue="POOL-328" type="fix" due-to="Lorenzo Solano Martinez">
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3956eedb/src/main/java/org/apache/commons/pool2/PooledObject.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/PooledObject.java b/src/main/java/org/apache/commons/pool2/PooledObject.java
index edf176f..06f50d3 100644
--- a/src/main/java/org/apache/commons/pool2/PooledObject.java
+++ b/src/main/java/org/apache/commons/pool2/PooledObject.java
@@ -167,6 +167,18 @@ public interface PooledObject<T> extends Comparable<PooledObject<T>> {
*/
void setLogAbandoned(boolean logAbandoned);
+// TODO: uncomment in 3.0 (API compatibility)
+// /**
+// * Configures the stack trace generation strategy based on whether or not fully
+// * detailed stack traces are required. When set to false, abandoned logs may
+// * only include caller class information rather than method names, line numbers,
+// * and other normal metadata available in a full stack trace.
+// *
+// * @param requireFullStackTrace the new configuration setting for abandoned object
+// * logging
+// */
+// void setRequireFullStackTrace(boolean requireFullStackTrace);
+
/**
* Record the current stack trace as the last time the object was used.
*/
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3956eedb/src/main/java/org/apache/commons/pool2/impl/AbandonedConfig.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/AbandonedConfig.java b/src/main/java/org/apache/commons/pool2/impl/AbandonedConfig.java
index fd430c5..24b27fb 100644
--- a/src/main/java/org/apache/commons/pool2/impl/AbandonedConfig.java
+++ b/src/main/java/org/apache/commons/pool2/impl/AbandonedConfig.java
@@ -17,11 +17,11 @@
package org.apache.commons.pool2.impl;
-import java.io.PrintWriter;
-
import org.apache.commons.pool2.TrackedUse;
import org.apache.commons.pool2.UsageTracking;
+import java.io.PrintWriter;
+
/**
* Configuration settings for abandoned object removal.
*
@@ -170,6 +170,43 @@ public class AbandonedConfig {
}
/**
+ * Determines whether or not to log full stack traces when logAbandoned is true.
+ * If disabled, then a faster method for logging stack traces with only class data
+ * may be used if possible.
+ *
+ * @since 2.5
+ */
+ private boolean requireFullStackTrace = true;
+
+ /**
+ * Indicates if full stack traces are required when {@link #getLogAbandoned() logAbandoned}
+ * is true. Defaults to true. Logging of abandoned objects requiring a full stack trace will
+ * generate an entire stack trace to generate for every object created. If this is disabled,
+ * a faster but less informative stack walking mechanism may be used if available.
+ *
+ * @return true if full stack traces are required for logging abandoned connections, or false
+ * if abbreviated stack traces are acceptable
+ * @see CallStack
+ * @since 2.5
+ */
+ public boolean getRequireFullStackTrace() {
+ return requireFullStackTrace;
+ }
+
+ /**
+ * Sets the flag to require full stack traces for logging abandoned connections when enabled.
+ *
+ * @param requireFullStackTrace indicates whether or not full stack traces are required in
+ * abandoned connection logs
+ * @see CallStack
+ * @see #getRequireFullStackTrace()
+ * @since 2.5
+ */
+ public void setRequireFullStackTrace(boolean requireFullStackTrace) {
+ this.requireFullStackTrace = requireFullStackTrace;
+ }
+
+ /**
* PrintWriter to use to log information on abandoned objects.
* Use of default system encoding is deliberate.
*/
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3956eedb/src/main/java/org/apache/commons/pool2/impl/CallStack.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/CallStack.java b/src/main/java/org/apache/commons/pool2/impl/CallStack.java
index 515b7bb..6caf6e1 100644
--- a/src/main/java/org/apache/commons/pool2/impl/CallStack.java
+++ b/src/main/java/org/apache/commons/pool2/impl/CallStack.java
@@ -24,7 +24,7 @@ import java.io.PrintWriter;
/**
* Strategy for obtaining and printing the current call stack. This is primarily useful for
* {@linkplain UsageTracking usage tracking} so that different JVMs and configurations can use more efficient strategies
- * for obtaining the current call stack.
+ * for obtaining the current call stack depending on metadata needs.
*
* @see CallStackUtils
* @since 2.4.3
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3956eedb/src/main/java/org/apache/commons/pool2/impl/CallStackUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/CallStackUtils.java b/src/main/java/org/apache/commons/pool2/impl/CallStackUtils.java
index fed4b3d..3b0bdda 100644
--- a/src/main/java/org/apache/commons/pool2/impl/CallStackUtils.java
+++ b/src/main/java/org/apache/commons/pool2/impl/CallStackUtils.java
@@ -55,10 +55,30 @@ public final class CallStackUtils {
* @param useTimestamp if true, interpret message as a SimpleDateFormat and print the created timestamp; otherwise,
* print message format literally
* @return a new CallStack
+ * @deprecated use {@link #newCallStack(String, boolean, boolean)}
*/
+ @Deprecated
public static CallStack newCallStack(final String messageFormat, final boolean useTimestamp) {
- return CAN_CREATE_SECURITY_MANAGER ? new SecurityManagerCallStack(messageFormat, useTimestamp) :
- new ThrowableCallStack(messageFormat, useTimestamp);
+ return newCallStack(messageFormat, useTimestamp, false);
+ }
+
+ /**
+ * Constructs a new {@link CallStack} using the fasted allowed strategy.
+ *
+ * @param messageFormat message (or format) to print first in stack traces
+ * @param useTimestamp if true, interpret message as a SimpleDateFormat and print the created timestamp;
+ * otherwise, print message format literally
+ * @param requireFullStackTrace if true, forces the use of a stack walking mechanism that includes full stack trace
+ * information; otherwise, uses a faster implementation if possible
+ * @return a new CallStack
+ * @since 2.5
+ */
+ public static CallStack newCallStack(final String messageFormat,
+ final boolean useTimestamp,
+ final boolean requireFullStackTrace) {
+ return CAN_CREATE_SECURITY_MANAGER && !requireFullStackTrace
+ ? new SecurityManagerCallStack(messageFormat, useTimestamp)
+ : new ThrowableCallStack(messageFormat, useTimestamp);
}
/**
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3956eedb/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java b/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java
index ffc07c5..a3ea902 100644
--- a/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java
+++ b/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java
@@ -42,10 +42,8 @@ public class DefaultPooledObject<T> implements PooledObject<T> {
private volatile long lastUseTime = createTime;
private volatile long lastReturnTime = createTime;
private volatile boolean logAbandoned = false;
- private final CallStack borrowedBy = CallStackUtils.newCallStack("'Pooled object created' " +
- "yyyy-MM-dd HH:mm:ss Z 'by the following code has not been returned to the pool:'", true);
- private final CallStack usedBy = CallStackUtils.newCallStack("The last code to use this object was:",
- false);
+ private volatile CallStack borrowedBy = NoOpCallStack.INSTANCE;
+ private volatile CallStack usedBy = NoOpCallStack.INSTANCE;
private volatile long borrowedCount = 0;
/**
@@ -276,4 +274,24 @@ public class DefaultPooledObject<T> implements PooledObject<T> {
this.logAbandoned = logAbandoned;
}
+ /**
+ * Configures the stack trace generation strategy based on whether or not fully
+ * detailed stack traces are required. When set to false, abandoned logs may
+ * only include caller class information rather than method names, line numbers,
+ * and other normal metadata available in a full stack trace.
+ *
+ * @param requireFullStackTrace the new configuration setting for abandoned object
+ * logging
+ * @since 2.5
+ */
+ // TODO: uncomment below in 3.0
+ // @Override
+ public void setRequireFullStackTrace(final boolean requireFullStackTrace) {
+ borrowedBy = CallStackUtils.newCallStack("'Pooled object created' " +
+ "yyyy-MM-dd HH:mm:ss Z 'by the following code has not been returned to the pool:'",
+ true, requireFullStackTrace);
+ usedBy = CallStackUtils.newCallStack("The last code to use this object was:",
+ false, requireFullStackTrace);
+ }
+
}
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3956eedb/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
index d743702..02d8d02 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
@@ -16,6 +16,15 @@
*/
package org.apache.commons.pool2.impl;
+import org.apache.commons.pool2.ObjectPool;
+import org.apache.commons.pool2.PoolUtils;
+import org.apache.commons.pool2.PooledObject;
+import org.apache.commons.pool2.PooledObjectFactory;
+import org.apache.commons.pool2.PooledObjectState;
+import org.apache.commons.pool2.SwallowedExceptionListener;
+import org.apache.commons.pool2.TrackedUse;
+import org.apache.commons.pool2.UsageTracking;
+
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
@@ -26,15 +35,6 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
-import org.apache.commons.pool2.ObjectPool;
-import org.apache.commons.pool2.PoolUtils;
-import org.apache.commons.pool2.PooledObject;
-import org.apache.commons.pool2.PooledObjectFactory;
-import org.apache.commons.pool2.PooledObjectState;
-import org.apache.commons.pool2.SwallowedExceptionListener;
-import org.apache.commons.pool2.TrackedUse;
-import org.apache.commons.pool2.UsageTracking;
-
/**
* A configurable {@link ObjectPool} implementation.
* <p>
@@ -337,6 +337,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
this.abandonedConfig.setRemoveAbandonedOnMaintenance(abandonedConfig.getRemoveAbandonedOnMaintenance());
this.abandonedConfig.setRemoveAbandonedTimeout(abandonedConfig.getRemoveAbandonedTimeout());
this.abandonedConfig.setUseUsageTracking(abandonedConfig.getUseUsageTracking());
+ this.abandonedConfig.setRequireFullStackTrace(abandonedConfig.getRequireFullStackTrace());
}
}
@@ -899,6 +900,10 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
final AbandonedConfig ac = this.abandonedConfig;
if (ac != null && ac.getLogAbandoned()) {
p.setLogAbandoned(true);
+ // TODO: in 3.0, this can use the method defined on PooledObject
+ if (p instanceof DefaultPooledObject<?>) {
+ ((DefaultPooledObject<T>) p).setRequireFullStackTrace(ac.getRequireFullStackTrace());
+ }
}
createdCount.incrementAndGet();
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3956eedb/src/main/java/org/apache/commons/pool2/impl/NoOpCallStack.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/NoOpCallStack.java b/src/main/java/org/apache/commons/pool2/impl/NoOpCallStack.java
new file mode 100644
index 0000000..b2d6a60
--- /dev/null
+++ b/src/main/java/org/apache/commons/pool2/impl/NoOpCallStack.java
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.pool2.impl;
+
+import java.io.PrintWriter;
+
+/**
+ * CallStack strategy using no-op implementations of all functionality. Can be used by default when abandoned object
+ * logging is disabled.
+ *
+ * @since 2.5
+ */
+public class NoOpCallStack implements CallStack {
+
+ public static final CallStack INSTANCE = new NoOpCallStack();
+
+ private NoOpCallStack() {
+ }
+
+ @Override
+ public boolean printStackTrace(PrintWriter writer) {
+ return false;
+ }
+
+ @Override
+ public void fillInStackTrace() {
+ // no-op
+ }
+
+ @Override
+ public void clear() {
+ // no-op
+ }
+}
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3956eedb/src/main/java/org/apache/commons/pool2/impl/ThrowableCallStack.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/ThrowableCallStack.java b/src/main/java/org/apache/commons/pool2/impl/ThrowableCallStack.java
index fe338ec..c10ed36 100644
--- a/src/main/java/org/apache/commons/pool2/impl/ThrowableCallStack.java
+++ b/src/main/java/org/apache/commons/pool2/impl/ThrowableCallStack.java
@@ -21,9 +21,9 @@ import java.text.DateFormat;
import java.text.SimpleDateFormat;
/**
- * CallStack strategy that uses the stack trace from a {@link Throwable}. While being the most portable method of
- * obtaining the current call stack, this is also the slowest way to do it. In environments where a new SecurityManager
- * can be created, it is preferred to use {@link SecurityManagerCallStack}.
+ * CallStack strategy that uses the stack trace from a {@link Throwable}. This strategy, while slower than the
+ * SecurityManager implementation, provides call stack method names and other metadata in addition to the call stack
+ * of classes.
*
* @see Throwable#fillInStackTrace()
* @since 2.4.3
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3956eedb/src/test/java/org/apache/commons/pool2/impl/NoOpCallStackTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/pool2/impl/NoOpCallStackTest.java b/src/test/java/org/apache/commons/pool2/impl/NoOpCallStackTest.java
new file mode 100644
index 0000000..83444cf
--- /dev/null
+++ b/src/test/java/org/apache/commons/pool2/impl/NoOpCallStackTest.java
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.pool2.impl;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+
+public class NoOpCallStackTest {
+ @Test
+ public void printStackTraceIsNoOp() throws Exception {
+ CallStack stack = NoOpCallStack.INSTANCE;
+ stack.fillInStackTrace();
+ StringWriter writer = new StringWriter();
+ stack.printStackTrace(new PrintWriter(writer));
+ Assert.assertEquals("", writer.toString());
+ }
+}
\ No newline at end of file