You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by fh...@apache.org on 2014/08/08 02:04:51 UTC
svn commit: r1616644 - in /tomcat/trunk/modules/jdbc-pool: doc/jdbc-pool.xml
src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
Author: fhanik
Date: Fri Aug 8 00:04:51 2014
New Revision: 1616644
URL: http://svn.apache.org/r1616644
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56318
Contribution by Danila Galimov
Ability to log statement creation stacks
Modified:
tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
Modified: tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml?rev=1616644&r1=1616643&r2=1616644&view=diff
==============================================================================
--- tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml (original)
+++ tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml Fri Aug 8 00:04:51 2014
@@ -608,6 +608,13 @@
and closes these statements when the connection is returned to the pool.
</p>
<attributes>
+ <attribute name="trace" required="false">
+ <p>(boolean as String) Enable tracing of unclosed statements.
+ When enabled and a connection is closed, and statements are not closed,
+ the interceptor will log all stack traces.
+ The default value is <code>false</code>.
+ </p>
+ </attribute>
</attributes>
</subsection>
<subsection name="org.apache.tomcat.jdbc.pool.interceptor.StatementCache">
Modified: tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java?rev=1616644&r1=1616643&r2=1616644&view=diff
==============================================================================
--- tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java (original)
+++ tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java Fri Aug 8 00:04:51 2014
@@ -19,6 +19,7 @@ package org.apache.tomcat.jdbc.pool.inte
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.jdbc.pool.ConnectionPool;
+import org.apache.tomcat.jdbc.pool.PoolProperties;
import org.apache.tomcat.jdbc.pool.PooledConnection;
import java.lang.ref.WeakReference;
@@ -26,6 +27,8 @@ import java.lang.reflect.Method;
import java.sql.Statement;
import java.util.LinkedList;
import java.util.List;
+import java.util.Map;
+
/**
* Keeps track of statements associated with a connection and invokes close upon {@link java.sql.Connection#close()}
* Useful for applications that dont close the associated statements after being done with a connection.
@@ -34,13 +37,15 @@ import java.util.List;
public class StatementFinalizer extends AbstractCreateStatementInterceptor {
private static final Log log = LogFactory.getLog(StatementFinalizer.class);
- protected List<WeakReference<Statement>> statements = new LinkedList<>();
-
+ protected List<WeakReference<StatementEntry>> statements = new LinkedList<>();
+
+ private boolean logCreationStack = false;
+
@Override
public Object createStatement(Object proxy, Method method, Object[] args, Object statement, long time) {
try {
if (statement instanceof Statement)
- statements.add(new WeakReference<>((Statement)statement));
+ statements.add(new WeakReference<>(new StatementEntry((Statement)statement)));
}catch (ClassCastException x) {
//ignore this one
}
@@ -50,25 +55,58 @@ public class StatementFinalizer extends
@Override
public void closeInvoked() {
while (statements.size()>0) {
- WeakReference<Statement> ws = statements.remove(0);
- Statement st = ws.get();
+ WeakReference<StatementEntry> ws = statements.remove(0);
+ StatementEntry st = ws.get();
if (st!=null) {
try {
- st.close();
+ st.getStatement().close();
} catch (Exception ignore) {
if (log.isDebugEnabled()) {
log.debug("Unable to closed statement upon connection close.",ignore);
}
}
+ if (logCreationStack) {
+ log.warn("Statement created, but was not closed at:", st.getAllocationStack());
+ }
}
}
}
@Override
+ public void setProperties(Map<String, PoolProperties.InterceptorProperty> properties) {
+ super.setProperties(properties);
+
+ PoolProperties.InterceptorProperty logProperty = properties.get("trace");
+ if (null != logProperty) {
+ logCreationStack = logProperty.getValueAsBoolean(logCreationStack);
+ }
+ }
+
+ @Override
public void reset(ConnectionPool parent, PooledConnection con) {
statements.clear();
super.reset(parent, con);
}
+ protected class StatementEntry {
+ private Statement statement;
+ private Throwable allocationStack;
+
+ public StatementEntry(Statement statement) {
+ this.statement = statement;
+ if (logCreationStack) {
+ this.allocationStack = new Throwable();
+ }
+ }
+
+ public Statement getStatement() {
+ return statement;
+ }
+
+ public Throwable getAllocationStack() {
+ return allocationStack;
+ }
+ }
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1616644 - in /tomcat/trunk/modules/jdbc-pool:
doc/jdbc-pool.xml src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
Posted by Filip Hanik <fi...@hanik.com>.
you are right, I will resolve it on Monday
thanks for the review
Filip
On Sun, Aug 10, 2014 at 5:38 PM, Konstantin Kolinko <kn...@gmail.com>
wrote:
> 2014-08-08 4:04 GMT+04:00 <fh...@apache.org>:
> > Author: fhanik
> > Date: Fri Aug 8 00:04:51 2014
> > New Revision: 1616644
> >
> > URL: http://svn.apache.org/r1616644
> > Log:
> > Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56318
> > Contribution by Danila Galimov
> > Ability to log statement creation stacks
> >
> > Modified:
> > tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml
> >
> tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
> >
> > Modified: tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml
> > URL:
> http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml?rev=1616644&r1=1616643&r2=1616644&view=diff
> >
> ==============================================================================
> > --- tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml (original)
> > +++ tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml Fri Aug 8 00:04:51
> 2014
> > @@ -608,6 +608,13 @@
> > and closes these statements when the connection is returned to
> the pool.
> > </p>
> > <attributes>
> > + <attribute name="trace" required="false">
> > + <p>(boolean as String) Enable tracing of unclosed statements.
> > + When enabled and a connection is closed, and statements are
> not closed,
> > + the interceptor will log all stack traces.
> > + The default value is <code>false</code>.
> > + </p>
> > + </attribute>
> > </attributes>
> > </subsection>
> > <subsection
> name="org.apache.tomcat.jdbc.pool.interceptor.StatementCache">
> >
> > Modified:
> tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
> > URL:
> http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java?rev=1616644&r1=1616643&r2=1616644&view=diff
> >
> ==============================================================================
> > ---
> tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
> (original)
> > +++
> tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
> Fri Aug 8 00:04:51 2014
> > @@ -19,6 +19,7 @@ package org.apache.tomcat.jdbc.pool.inte
> > import org.apache.juli.logging.Log;
> > import org.apache.juli.logging.LogFactory;
> > import org.apache.tomcat.jdbc.pool.ConnectionPool;
> > +import org.apache.tomcat.jdbc.pool.PoolProperties;
> > import org.apache.tomcat.jdbc.pool.PooledConnection;
> >
> > import java.lang.ref.WeakReference;
> > @@ -26,6 +27,8 @@ import java.lang.reflect.Method;
> > import java.sql.Statement;
> > import java.util.LinkedList;
> > import java.util.List;
> > +import java.util.Map;
> > +
> > /**
> > * Keeps track of statements associated with a connection and invokes
> close upon {@link java.sql.Connection#close()}
> > * Useful for applications that dont close the associated statements
> after being done with a connection.
> > @@ -34,13 +37,15 @@ import java.util.List;
> > public class StatementFinalizer extends
> AbstractCreateStatementInterceptor {
> > private static final Log log =
> LogFactory.getLog(StatementFinalizer.class);
> >
> > - protected List<WeakReference<Statement>> statements = new
> LinkedList<>();
> > -
> > + protected List<WeakReference<StatementEntry>> statements = new
> LinkedList<>();
>
>
> 1. This approach is broken.
>
> Essentially this change breaks StatementFinalizer.
>
> A weak reference to a Statement is OK, because there are hard
> references to that statement object elsewhere.
>
> A weak reference to a StatementEntry class is not OK, because it is
> the only reference to it. It is immediately eligible to be
> garbage-collected.
>
>
> > +
> > + private boolean logCreationStack = false;
> > +
> > @Override
> > public Object createStatement(Object proxy, Method method, Object[]
> args, Object statement, long time) {
> > try {
> > if (statement instanceof Statement)
> > - statements.add(new
> WeakReference<>((Statement)statement));
> > + statements.add(new WeakReference<>(new
> StatementEntry((Statement)statement)));
> > }catch (ClassCastException x) {
> > //ignore this one
> > }
> > @@ -50,25 +55,58 @@ public class StatementFinalizer extends
> > @Override
> > public void closeInvoked() {
> > while (statements.size()>0) {
> > - WeakReference<Statement> ws = statements.remove(0);
> > - Statement st = ws.get();
> > + WeakReference<StatementEntry> ws = statements.remove(0);
> > + StatementEntry st = ws.get();
> > if (st!=null) {
> > try {
> > - st.close();
> > + st.getStatement().close();
> > } catch (Exception ignore) {
> > if (log.isDebugEnabled()) {
> > log.debug("Unable to closed statement upon
> connection close.",ignore);
> > }
> > }
> > + if (logCreationStack) {
> > + log.warn("Statement created, but was not closed
> at:", st.getAllocationStack());
> > + }
> > }
> > }
> > }
> >
> > @Override
> > + public void setProperties(Map<String,
> PoolProperties.InterceptorProperty> properties) {
> > + super.setProperties(properties);
> > +
> > + PoolProperties.InterceptorProperty logProperty =
> properties.get("trace");
> > + if (null != logProperty) {
> > + logCreationStack =
> logProperty.getValueAsBoolean(logCreationStack);
> > + }
> > + }
> > +
> > + @Override
> > public void reset(ConnectionPool parent, PooledConnection con) {
> > statements.clear();
> > super.reset(parent, con);
> > }
> >
> > + protected class StatementEntry {
> > + private Statement statement;
> > + private Throwable allocationStack;
>
> 2. Missing "final".
>
> > +
> > + public StatementEntry(Statement statement) {
> > + this.statement = statement;
> > + if (logCreationStack) {
> > + this.allocationStack = new Throwable();
> > + }
> > + }
> > +
> > + public Statement getStatement() {
> > + return statement;
> > + }
> > +
> > + public Throwable getAllocationStack() {
> > + return allocationStack;
> > + }
> > + }
> > +
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
Re: svn commit: r1616644 - in /tomcat/trunk/modules/jdbc-pool:
doc/jdbc-pool.xml src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-08-08 4:04 GMT+04:00 <fh...@apache.org>:
> Author: fhanik
> Date: Fri Aug 8 00:04:51 2014
> New Revision: 1616644
>
> URL: http://svn.apache.org/r1616644
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56318
> Contribution by Danila Galimov
> Ability to log statement creation stacks
>
> Modified:
> tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml
> tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
>
> Modified: tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml
> URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml?rev=1616644&r1=1616643&r2=1616644&view=diff
> ==============================================================================
> --- tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml (original)
> +++ tomcat/trunk/modules/jdbc-pool/doc/jdbc-pool.xml Fri Aug 8 00:04:51 2014
> @@ -608,6 +608,13 @@
> and closes these statements when the connection is returned to the pool.
> </p>
> <attributes>
> + <attribute name="trace" required="false">
> + <p>(boolean as String) Enable tracing of unclosed statements.
> + When enabled and a connection is closed, and statements are not closed,
> + the interceptor will log all stack traces.
> + The default value is <code>false</code>.
> + </p>
> + </attribute>
> </attributes>
> </subsection>
> <subsection name="org.apache.tomcat.jdbc.pool.interceptor.StatementCache">
>
> Modified: tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java?rev=1616644&r1=1616643&r2=1616644&view=diff
> ==============================================================================
> --- tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java (original)
> +++ tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java Fri Aug 8 00:04:51 2014
> @@ -19,6 +19,7 @@ package org.apache.tomcat.jdbc.pool.inte
> import org.apache.juli.logging.Log;
> import org.apache.juli.logging.LogFactory;
> import org.apache.tomcat.jdbc.pool.ConnectionPool;
> +import org.apache.tomcat.jdbc.pool.PoolProperties;
> import org.apache.tomcat.jdbc.pool.PooledConnection;
>
> import java.lang.ref.WeakReference;
> @@ -26,6 +27,8 @@ import java.lang.reflect.Method;
> import java.sql.Statement;
> import java.util.LinkedList;
> import java.util.List;
> +import java.util.Map;
> +
> /**
> * Keeps track of statements associated with a connection and invokes close upon {@link java.sql.Connection#close()}
> * Useful for applications that dont close the associated statements after being done with a connection.
> @@ -34,13 +37,15 @@ import java.util.List;
> public class StatementFinalizer extends AbstractCreateStatementInterceptor {
> private static final Log log = LogFactory.getLog(StatementFinalizer.class);
>
> - protected List<WeakReference<Statement>> statements = new LinkedList<>();
> -
> + protected List<WeakReference<StatementEntry>> statements = new LinkedList<>();
1. This approach is broken.
Essentially this change breaks StatementFinalizer.
A weak reference to a Statement is OK, because there are hard
references to that statement object elsewhere.
A weak reference to a StatementEntry class is not OK, because it is
the only reference to it. It is immediately eligible to be
garbage-collected.
> +
> + private boolean logCreationStack = false;
> +
> @Override
> public Object createStatement(Object proxy, Method method, Object[] args, Object statement, long time) {
> try {
> if (statement instanceof Statement)
> - statements.add(new WeakReference<>((Statement)statement));
> + statements.add(new WeakReference<>(new StatementEntry((Statement)statement)));
> }catch (ClassCastException x) {
> //ignore this one
> }
> @@ -50,25 +55,58 @@ public class StatementFinalizer extends
> @Override
> public void closeInvoked() {
> while (statements.size()>0) {
> - WeakReference<Statement> ws = statements.remove(0);
> - Statement st = ws.get();
> + WeakReference<StatementEntry> ws = statements.remove(0);
> + StatementEntry st = ws.get();
> if (st!=null) {
> try {
> - st.close();
> + st.getStatement().close();
> } catch (Exception ignore) {
> if (log.isDebugEnabled()) {
> log.debug("Unable to closed statement upon connection close.",ignore);
> }
> }
> + if (logCreationStack) {
> + log.warn("Statement created, but was not closed at:", st.getAllocationStack());
> + }
> }
> }
> }
>
> @Override
> + public void setProperties(Map<String, PoolProperties.InterceptorProperty> properties) {
> + super.setProperties(properties);
> +
> + PoolProperties.InterceptorProperty logProperty = properties.get("trace");
> + if (null != logProperty) {
> + logCreationStack = logProperty.getValueAsBoolean(logCreationStack);
> + }
> + }
> +
> + @Override
> public void reset(ConnectionPool parent, PooledConnection con) {
> statements.clear();
> super.reset(parent, con);
> }
>
> + protected class StatementEntry {
> + private Statement statement;
> + private Throwable allocationStack;
2. Missing "final".
> +
> + public StatementEntry(Statement statement) {
> + this.statement = statement;
> + if (logCreationStack) {
> + this.allocationStack = new Throwable();
> + }
> + }
> +
> + public Statement getStatement() {
> + return statement;
> + }
> +
> + public Throwable getAllocationStack() {
> + return allocationStack;
> + }
> + }
> +
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org