tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Richard S. Huntrods" <huntr...@nucleus.com>
Subject Re: Would like to monitor memory use offline
Date Sat, 09 Aug 2008 19:02:04 GMT
> Richard,
>
> Richard S. Huntrods wrote:
> |    public static Vector listLookup(String table) {
> | //        Connection connection = null; // connection is managed by a
> | connection pool
>
> So, is 'connection' a local or not?
It's part of my code (I use my own connection pool class) but not to the 
DBMS code. Only if you can't get a connection from the pool 
(connection==null) do you create a local one for this method.

If the connection from the pool is OK, you use it then release it back 
to the pool. If it's local (because you could not get one from the pool) 
then you close it in the finally block.
>
> |        Statement statement = null;
> |        ResultSet resultSet = null;
> |
> |        // connection is managed by a connection pool
> |        try {
> |            if(connection == null) {
> |                getConnection();
> |            }
>
> If 'connection' is a local, what does 'getConnection' do?
Gets a connection from the pool. I've never seen a case in the 
production code when the pool failed to provide a connection. The local 
is just a fallback.
>
> If 'connection' is /not/ a local, then you are asking for threading 
> trouble.
>
> |            if(connection != null) {
> |                statement = connection.createStatement();
> |                resultSet = statement.executeQuery("SELECT * FROM " +
> | table + " ORDER BY idNumber");
> |                if(resultSet != null) {
> |                    /* process resultSet */
> |                    }
> |                    resultSet.close();
> |                }
> |                statement.close();
> |            }
> |            return /* results Vector */
> |        }
> |        catch (SQLException sqle) {
> |        /* debug messages */
> |        }
> |        catch (Exception e) {
> |        /* debug messages */
> |        }
> |        finally {
> |        /* debug messages */
> |            connection = null;
>
> You need to have statement.close() and resultSet.close() in here (in the
> finally block), not up above. Also, most database connection pools
> require that you call connection.close() to return the connection to the
> pool. Are you ever calling connection.close()?
Um, sorry to disagree, but that is not really correct and also does not 
touch my problem.

I am already closing the resultSet and statement in the "normal 
execution" part of my code. At no time during the execution of this code 
in these tests was an exception thrown in this method, so *I was now 
correctly closing the statement and resultSet. YET IN SPITE OF THIS the 
verisions of the connector after 3.1.10 FAIL to release the Field objects.

The other reason NOT to put close() in the fianlly block is that the 
close() methods can throw and exception. It is very bad programming 
practice to put "exception throwing code' in a finally block. Sure, you 
could use another "try-catch" structure for the close() statements, but 
really - you should have already closed the stuff in the normal 
execution (as I did).

I did state one thing in error - the finally block (of couse) ALWAYS 
gets called, so you only want code in that block that must be done after 
all else has happened, but you also don't really want to be repeating 
code that should go elsewhere (like the close()).
>
>
> Also, which connection pool do you use?
My own.

Yea - the problem may be in the connection pool that I wrote, but I 
still find it odd that closing the resultSet() and statement() has no 
effect using connectors from 3.1.10 and on. Connector 3.0.17b and prior 
work just fine and release the objects.

Cheers,

-Richard
>
> |        }
> |        return null;
> |    }
> |
> | NOTE: by checking debug logs, I can tell that during normal use there
> | are NO exceptions occurring, and the finally is NOT getting called - 
> yet
> | the memory leak is still going on.
>
> The finally block had /better/ be called: it's a requirement of the JVM
> and the language.
>
> - -chris 

---------------------------------------------------------------------
To start a new topic, e-mail: users@tomcat.apache.org
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Mime
View raw message