2014-08-08 4:04 GMT+04:00  <fha...@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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to