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 <boa...@gmail.com> Authored: Fri Nov 10 11:37:38 2017 -0600 Committer: Matt Sicker <boa...@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