DatabaseMetaData.getTables() Memory Leak?

[Peter Borissow]

Taro/Grace-

      I found a pretty significant memory leak when calling this routine:

            java.sql.DatabaseMetaData dbmd = conn.getMetaData();
            java.sql.ResultSet rs = dbmd.getTables(null,null,null,new String[]{"TABLE"});

            rs.close();
            rs = null;

I found that this chuck of code eats up approximately 10KB of RAM and is never released. After several thousand iterations (e.g. web app), this can really add up.

After a little digging, I *think* I have tracked down the source of the problem. In org.sqlite.MetaData.getTables() a statement is created but never closed. Example:

            return conn.createStatement().executeQuery(sql);

For testing purposes, I updated the getTables() method so that instead of returning the resultset, I did this:

                java.sql.Statement stmt = conn.createStatement();
                java.sql.ResultSet rs = stmt.executeQuery(sql);
               

                while (rs.next()) {

                }
                rs.close();
                rs = null;
                stmt.close();

                return null;

By explicitly closing the statement, I eliminated the memory leak. You can test it yourself by commenting out stmt.close(). Of course, returning a null recordset is not a solution. And you can't return a resultset after closing the statement.

So what's the solution?

Should there be some sort of conditional logic inside org.sqlite.RS.close() to call stmt.close()?

Thanks,

Peter

[gmseed]

Hi

I was interested in your comment "you can't return a resultset after closing the statement."

I just set up a test program that inserts a call to close() after a result set is returned. After the call to close() I then iterator through the result set and my example works the same irrespective of the call to close:

            rs   = stmt.executeQuery(selectQuery);

            stmt.close(); /*** call to Statement.close() before operate on ResultSet ***/

            while (rs.next())

            {   

                int id = rs.getInt("id");

                byte[] byteArray = rs.getBytes("bytes");

                System.out.println("i: " + id + ", byteArray read from db: " + ToStringBuffer.list(byteArray, "[", ", ","}"));          

            }

Thus, in your case what is wrong with:

ResultSet getTables(/* ... */) {

...

Statement stmt = createStatement();

...

ResultSet rs = ..;

...

stmt.close();

return rs;

}

Graham

[Peter Borissow]

Yes. You are right.

Also, I was too hasty to point out that stmt.close() would solve the issue. Here's a test script you can use to verify the issue. You don't need access to the SQLite JDBC source code to test this.

PS. I'm running on Windows 7x64 with Java 1.6 using sqlite-jdbc-3.7.15

...............

            java.sql.Driver driver = (java.sql.Driver) Class.forName("org.sqlite.JDBC").newInstance();
            java.sql.Connection conn = driver.connect(url, config.toProperties());

            java.sql.DatabaseMetaData dbmd = conn.getMetaData();
            for (int i=0; i<150000; i++){
                try{

                    java.sql.ResultSet rs = dbmd.getTables(null,null,null,new String[]{"TABLE"});

                    while (rs.next()) {

                    }
                    rs.close();
                    rs = null;

                }

                catch(Exception e){
                    e.printStackTrace();
                }
            }
            conn.close();

................

    public synchronized java.sql.ResultSet getTables(java.sql.Connection conn) throws SQLException {
        //checkOpen();

        String[] types = new String[]{"TABLE"};

        String t = null;
        t = (t == null || "".equals(t)) ? "%" : t.toUpperCase();

        String sql = "select" + " null as TABLE_CAT," + " null as TABLE_SCHEM," + " name as TABLE_NAME,"
                + " upper(type) as TABLE_TYPE," + " null as REMARKS," + " null as TYPE_CAT," + " null as TYPE_SCHEM,"
                + " null as TYPE_NAME," + " null as SELF_REFERENCING_COL_NAME," + " null as REF_GENERATION"
                + " from (select name, type from sqlite_master union all"
                + "       select name, type from sqlite_temp_master)" + " where TABLE_NAME like '" + escape(t) + "'";

        if (types != null) {
            sql += " and TABLE_TYPE in (";
            for (int i = 0; i < types.length; i++) {
                if (i > 0) {
                    sql += ", ";
                }
                sql += "'" + types[i].toUpperCase() + "'";
            }
            sql += ")";
        }

        sql += ";";

        //return conn.createStatement().executeQuery(sql);

        java.sql.Statement stmt = conn.createStatement();
        java.sql.ResultSet rs = stmt.executeQuery(sql);

        stmt.close();
        return rs;
    }


    private String escape(final String val) {
        // TODO: this function is ugly, pass this work off to SQLite, then we
        //       don't have to worry about Unicode 4, other characters needing
        //       escaping, etc.
        int len = val.length();
        StringBuilder buf = new StringBuilder(len);
        for (int i = 0; i < len; i++) {
            if (val.charAt(i) == '\'') {
                buf.append('\'');
            }
            buf.append(val.charAt(i));
        }
        return buf.toString();
    }

---

From: gmseed <gms...@gmail.com>
To: xer...@googlegroups.com
Cc: Peter Borissow <peter.b...@yahoo.com>
Sent: Friday, May 10, 2013 12:23 PM
Subject: Re: DatabaseMetaData.getTables() Memory Leak?

Hi

I was interested in your comment "you can't return a resultset after closing the statement."

I just set up a test program that inserts a call to close() after a result set is returned. After the call to close() I then iterator through the result set and my example works the same irrespective of the call to close:

            rs   = stmt.executeQuery(selectQuery);

            stmt.close(); /*** call to Statement.close() before operate on ResultSet ***/

            while (rs.next())

            {   

                int id = rs.getInt("id");

                byte[] byteArray = rs.getBytes("bytes");

                System.out.println("i: " + id + ", byteArray read from db: " + ToStringBuffer.list(byteArray, "[", ", ","}"));          

            }

Thus, in your case what is wrong with:

ResultSet getTables(/* ... */) {

...

Statement stmt = createStatement();

...

ResultSet rs = ..;

...

stmt.close();

return rs;

}

Graham

On Friday, May 10, 2013 4:53:44 PM UTC+1, Peter wrote:

Taro/Grace-

      I found a pretty significant memory leak when calling this routine:

            java.sql.DatabaseMetaData dbmd = conn.getMetaData();
            java.sql.ResultSet rs = dbmd.getTables(null,null,null, new String[]{"TABLE"});

            rs.close();
            rs = null;

I found that this chuck of code eats up approximately 10KB of RAM and is never released. After several thousand iterations (e.g. web app), this can really add up.

After a little digging, I *think* I have tracked down the source of the problem. In org.sqlite.MetaData.getTables( ) a statement is created but never closed. Example:

[Peter Borissow]

*** CORRECTION ***

Definitely need to close the statement to avoid the memory leak!

Using my test script, instead of this:

java.sql.ResultSet rs = dbmd.getTables(null,null,null,new String[]{"TABLE"});

Use this:

java.sql.ResultSet rs = getTables(conn);


If the statement is closed in getTables(), there is no memory leak.


BUT...

The resultset is empty if I close the statement in getTables(). Can anyone confirm? Maybe Graham?


Sorry for all the noise. Its been a long day...

Peter

---

From: Peter Borissow <peter.b...@yahoo.com>
To: gmseed <gms...@gmail.com>; "xer...@googlegroups.com" <xer...@googlegroups.com>
Sent: Friday, May 10, 2013 12:57 PM

[gmseed]

Hi

Just did a bit of googling and found the following on StackOverflow:

http://stackoverflow.com/questions/4507440/must-jdbc-resultsets-and-statements-be-closed-separately-although-the-connection

Checkout the post having 12 recommendations.

It reiterates what the Java JavaDoc states:

"When a Statement object is closed, its current ResultSet object, if one exists, is also closed."

so it looks like you were correct in stating that closing a statement associated with a result-set effects the result set.

This is not what I see in my in-memory example.

Graham

[Peter Borissow]

Sounds like you might have found a bug in the in-memory database :-(

Peter

---

From: gmseed <gms...@gmail.com>
To: xer...@googlegroups.com

Cc: gmseed <gms...@gmail.com>; Peter Borissow <peter.b...@yahoo.com>
Sent: Friday, May 10, 2013 3:22 PM

Subject: Re: DatabaseMetaData.getTables() Memory Leak?

Hi

Just did a bit of googling and found the following on StackOverflow:

http://stackoverflow.com/questions/4507440/must-jdbc-resultsets-and-statements-be-closed-separately-although-the-connection

Checkout the post having 12 recommendations.

It reiterates what the Java JavaDoc states:

"When a Statement object is closed, its current ResultSet object, if one exists, is also closed."

so it looks like you were correct in stating that closing a statement associated with a result-set effects the result set.

This is not what I see in my in-memory example.

Graham

On Friday, May 10, 2013 5:57:48 PM UTC+1, Peter wrote:

Yes. You are right.

Also, I was too hasty to point out that stmt.close() would solve the issue. Here's a test script you can use to verify the issue. You don't need access to the SQLite JDBC source code to test this.

PS. I'm running on Windows 7x64 with Java 1.6 using sqlite-jdbc-3.7.15

...............

            java.sql.Driver driver = (java.sql.Driver) Class.forName("org.sqlite. JDBC").newInstance();

            java.sql.Connection conn = driver.connect(url, config.toProperties());

            java.sql.DatabaseMetaData dbmd = conn.getMetaData();
            for (int i=0; i<150000; i++){
                try{

                    java.sql.ResultSet rs = dbmd.getTables(null,null,null, new String[]{"TABLE"});

[Grace B]

This issue is now fixed https://bitbucket.org/xerial/sqlite-jdbc/issue/73/databasemetadatagettables-memory-leak

[Peter Borissow]

Fantastic! Can't wait to get another snapshot to test it out.

BTW, I looked at the diff files. Is the autoClose parameter being used in Stmt.executeQuery(String sql, boolean autoClose)?

---

From: Grace B <grace.b...@gmail.com>

To: xer...@googlegroups.com
Cc: gmseed <gms...@gmail.com>; Peter Borissow <peter.b...@yahoo.com>

Sent: Saturday, May 11, 2013 5:47 PM

Subject: Re: DatabaseMetaData.getTables() Memory Leak?

[Grace B]

Another good catch. The parameter is now used.