MySql too many connections

11,676

Solution 1

With your approach, the connection will never be closed if any exception is been thrown before the conn.close() is called. You need to acquire it (and the statement and resultset) in a try block and close it in the finally block. Any code in finally will always be executed regardless of an exception is been thrown or not. With this you can ensure that the expensive resources will be closed.

Here's a rewrite:

public int getSiteIdFromName(String name, String company) throws DataAccessException, java.sql.SQLException {
    Connection conn = null;
    Statement smt = null;
    ResultSet rs = null;
    int id = 0;
    try {
        conn = this.getSession().connection();
        smt = conn.createStatement();
        String query = "SELECT id FROM site WHERE name='" + name + "' and company_id='" + company + "'";
        rs = smt.executeQuery(query);
        rs.next();
        id = rs.getInt("id");
    } finally {
        if (rs != null) try { rs.close(); } catch (SQLException logOrIgnore) {}
        if (smt != null) try { smt.close(); } catch (SQLException logOrIgnore) {}
        if (conn != null) try { conn.close(); } catch (SQLException logOrIgnore) {}
    }
    return id;
}

That said, this code is sensitive to SQL injection attacks. Use a PreparedStatement instead of Statement.

See also:

Solution 2

One possible flow in which this code can leak connection is:

  1. Stmt.executeQuery() results empty resultset
  2. You do not check whether rs.next() returns true or false
  3. rs.getInt("id") throws exception as there is not current row in resultset
  4. conn.close() is skipped

Do the following:

  1. Make rs.getInt() conditional on rs.next()
  2. Close the connection in finally block and do all the data access within try block

Edit:

Also it's a good idea to log all exceptions somewhere so that you have a good starting point in your troubleshooting.

Share:
11,676
MichaelMcCabe
Author by

MichaelMcCabe

Studied Computer Science at Cardiff University. Have worked for many years as a Java and .NET developer for companies such as Red Hat, BAE Systems, and now leading a team of developers in south wales UK. Recently starting to learn MVC and more client side technologies to improve the software I write

Updated on June 07, 2022

Comments

  • MichaelMcCabe
    MichaelMcCabe almost 2 years

    I hate to bring up a question which is widely asked on the web, but I cant seem to solve it.

    I started a project a while back and after a month of testing, I hit a "Too many connections" error. I looked into it, and "Solved" it by increasing the max_connections. This then worked.

    Since then more and more people started to use it, and it hit again. When I am the only user on the site, i type "show processlist" and it comes up with about 50 connections which are still open (saying "Sleep" in the command). Now, I dont know enough to speculate why these are open, but in my code I tripple checked and every connection I open, I close.

    ie.

    public int getSiteIdFromName(String name, String company)throws DataAccessException,java.sql.SQLException{
    
    Connection conn = this.getSession().connection();
    Statement smt = conn.createStatement();
    ResultSet rs=null;
    String query="SELECT id FROM site WHERE name='"+name+"' and company_id='"+company+"'";
    
    rs=smt.executeQuery(query);
    rs.next();
    
    int id=rs.getInt("id");
    
    rs.close();
    smt.close();
    conn.close();
    return id;
    }
    

    Every time I do something else on the site, another load of connections are opened and not closed. Is there something wrong with my code? and if not, what could be the problem?