11 Replies Latest reply on May 23, 2003 11:28 AM by juhalindfors

    Closing a statement you left open, please do your own

    shortpasta

      Sample log statement:
      <Closing a statement you left open, please do your own housekeeping>

      In my code I do this:
      Connection connection = null;
      try {
      connection = JBoss.getConnection ();
      final Statement statement = connection.createStatement ();
      //use statement
      //do NOT close statement
      }
      finally {
      connection.close ();
      }


      This java.sql.Connection wrapper logs a WARNing when a statement is left open. However the JDBC spec says that Connection.close () will close any open statement(s) it created, so as long as you do connection.close () you are ok, and if you don't close it, the spec says that the connection should close itself upon GC.

      The spec left it to the developer to decide whether to close statements immediately or just let the connection do it when it is closed, so why is WrapperConnection basically forcing me to close all my statements? anybody knows how to get around this?

        • 1. Re: Closing a statement you left open, please do your own
          jonlee

          You are not really opening or closing a connection in this instance. You are obtaining a shared connection via a datasource (connection pool). The connection.close() is merely returning the connection to the connection pool where some other component will pick up this open connection and use it for executing its own statements.

          The whole point of having a connection pool is that opening and closing connections are actually expensive tasks (resources and time) so you don't really want to do this every time you want to access the datasource (usually a database).

          However, this means that you should properly close a statement to force an execution.

          This is a warning message. You don't need to explicitly close the statement but it means that it might not actually be executed at the time you expect. Hope that helps.

          • 2. Re: Closing a statement you left open, please do your own
            shortpasta

            The problem is still that i'm getting a WARN when I should NOT get a WARN. Basically, this behavior gets in the way of log file monitoring.

            I'll counter your argument with this example from the apache database connection pool library. This method executes from a connection's close (). As required by the spec they properly close all open statements. The connection is then moved back to the pool

            protected void passivate() throws SQLException {
            _closed = true;
            // The JDBC spec requires that a Connection close any open
            // Statement's when it is closed.
            List statements = getTrace();
            if( statements != null) {
            Statement[] set = new Statement[statements.size()];
            statements.toArray(set);
            for (int i = 0; i < set.length; i++) {
            set..close();
            }
            clearTrace();
            }
            setLastUsed(0);
            if(_conn instanceof DelegatingConnection) {
            ((DelegatingConnection)_conn).passivate();
            }
            }

            • 3. Re: Closing a statement you left open, please do your own
              jonlee

              Based on your comments, I went back through the JDBC reference, 2nd ed. to see if there was anything definitive on the matter. Unfortunately, the spec is very non-commital on the matter of DataSource and PooledConnection. A pooled connection close is only required at the end of the lifecycle of the pool, closing the underlying connection. It doesn't make any conditionals on what is done when a connection is returned to a pool. My guess is that there are some complexities with regards to handling chained transactions that the original JDBC specs did not cover since it was before J2EE prominence.

              There is an onus however, on application programmers to explicitly release resources - to explicitly close statements and close connections. See http://java.sun.com/j2se/1.4.1/docs/guide/jdbc/getstart/connection.html, section 2.1.9 on freeing DBMS resources.

              Now, I'm not taking a stance on whether the implementation is right or wrong - that you'd have to argue with the developers. I'm just stating the observed operation, the lack of definitive specs for operation and the probably rationale. Previous JBoss implementations did not complain with leaving statements open. But with these guidelines, I would suggest that a warning might be OK if Sun recommends that in best practice, programmers explicitly close statements and connections.

              Other than lobbying an implementation change, the only recourse is to filter WARNing messages I think.

              • 4. Re: Closing a statement you left open, please do your own

                We are seeing these warnings on closing the connection in situations where we are sure we have closed the result sets and (prepared) statements.

                However, we are also seeing other warnings elsewhere stating that we are trying to return an unrecognised connection to the pool (sorry I don't have the exact text handy).

                Makes me think that there is some subtle bug in the JBoss connection pool code - maybe recording the wrong connections when handing them out.

                If I can produce a test case, I'll post it here.

                • 5. Re: Closing a statement you left open, please do your own
                  shortpasta

                  You are right, short of a connection pool change the only thing to do is to filter out the WARN statements. I would do that by setting the log4j log level for the WrapperConnection to ERROR.

                  Unfortunately the problem with that is that i will lose any other valuable WARNings that the current/future WrappedConnection code does/will log.

                  The last alternative is for me to close all statements right after i'm done using them. As you can guess, we have many of those.

                  Also the majority of our direct JDBC code uses only 1 statement. Closing it becomes just an extra drag!

                  We have a ConnectionTemplate class that basically contains this code.
                  Connection connection = null;
                  try {
                  connection = pool.getConnection ();
                  run (connection);
                  }
                  finally {
                  connection.close ();
                  }

                  Anybody that wants to have a connection does this to use the connection.
                  new ConnectionTemplate () {
                  public void run (final Connection connection) {
                  // use connection
                  }
                  }.run ();

                  I have workarounds as you can probably argue by looking at this sample code, but in one way or another none of them are "clean".

                  At this point i'm leaning toward the logLevel change.

                  • 6. Re: Closing a statement you left open, please do your own
                    yoyodyne

                    Yup, there are several issues here.

                    (1)I think the JBoss behaviour is wrong cuz i am not able to implement the page-by-page iterator pattern. i have a stateful session bean which has method1 which obtains a connection from the pool and uses it to execute the query and the result is a scrollable result set. Now i have to keep my connection and resultset open for subsequent method invocations on the stateful session bean for the purpose of scrolling thru the resultset which was generated as a result of the first method. however, the moment i exit method1, jboss closes my connection and the subsequent method invocations on the stateful session bean fail.

                    (2) U close the connection, jboss throws the warning. U dont close the connection, still jboss throws the warning. so what does JBoss expect us to do when it throws this warning.

                    (3) Also it does not allow me to close prepared statements.

                    • 7. Re: Closing a statement you left open, please do your own
                      yoyodyne

                      Yup, there are several issues here.

                      (1)I think the JBoss behaviour is wrong cuz i am not able to implement the page-by-page iterator pattern. i have a stateful session bean which has method1 which obtains a connection from the pool and uses it to execute the query and the result is a scrollable result set. Now i have to keep my connection and resultset open for subsequent method invocations on the stateful session bean for the purpose of scrolling thru the resultset which was generated as a result of the first method. however, the moment i exit method1, jboss closes my connection and the subsequent method invocations on the stateful session bean fail.

                      (2) U close the connection, jboss throws the warning. U dont close the connection, still jboss throws the warning. so what does JBoss expect us to do when it throws this warning.

                      (3) Also it does not allow me to close prepared statements.

                      • 8. Re: Closing a statement you left open, please do your own
                        yoyodyne

                        Please ignore issue(2) above.

                        To elaborate a bit more on (3), when i try to close the prepared statement, it throws some exception. To avoid the exception, i choose not to close the prepared statement. In that case, JBoss issues the "Closing a statement you left open, please do your own housekeeping" warning. So what exactly does JBoss expect us to do here. Please note that here my prepared statement is created as follows -
                        ps = conn.prepareStatement( sql, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY );

                        • 9. Re: Closing a statement you left open, please do your own
                          jonlee

                          With which version are you having these problems? I'm still on 3.2.0 and have had no problems with closing prepared statements. It's our primary mode for writing to the DB when we code our own SQL.

                          We've also not noticed any issues when we explicitly close statements.

                          • 10. Re: Closing a statement you left open, please do your own
                            yoyodyne

                            I am also on JBoss3.2.0. I dont have any issues while closing regular prepared statements. I think the issue is when i use ResultSet.SCROLL_INSENSITIVE while creating the prepared statement.

                            • 11. Re: Closing a statement you left open, please do your own

                              If you think there's an issue, create a reproducible test case and attach it to your bug report at sf.net. I'm sure David will have a look at it.