tremon
tremon

Reputation: 85

Java SQL memory leak

I'm facing an issue where I have a java application running on a server, and it starts growing in memory until eventually the server cannot handle it anymore.

This is some sort of memory leak / resource leak problem, which I thought was extremely rare in Java due to the garbage collection. I guess something is being referenced and never used, so the garbage collector does not collect it.

The problem is that the size in memory grows so slowly that I'm not able to debug it properly (it may take two weeks to make the server unusable). I'm using java + mysql-connector, and I'm sure the memory leak is caused by something related to the database connection.

Here is how I connect to the database:

 private static Connection connect(){
    try {
        Connection conn = null;
        conn =  DriverManager.getConnection("jdbc:mysql://localhost:3306/database","client","password");
        return conn;
    }catch(SQLException ex){
        System.out.println("SQLException: " + ex.getMessage());
        System.out.println("SQLState: " + ex.getSQLState());
        System.out.println("VendorError: " + ex.getErrorCode());
        return null;
    }
}
public static Connection getConnection(){
    try {
        if (connection == null || connection.isClosed()) connection = connect();
        return connection;

    }catch (SQLException exception){
        System.out.println("exception trying to connect to the database");
        return null;
    }
}

I can't find any possible problem here, but who knows!

Here's how I retrieve information from the database:

 public void addPoints(long userId,int cantidad){
    try {
        if(DatabaseConnector.getConnection()!=null) {
            PreparedStatement stm = DatabaseConnector.getConnection().prepareStatement("UPDATE users SET points = points + ? WHERE id = ? ");
            stm.setLong(2, userId);
            stm.setInt(1, cantidad);
            if(stm.executeUpdate()==0){ //user doesn't have any point records in the database yet
                PreparedStatement stm2 = DatabaseConnector.getConnection().prepareStatement("INSERT INTO users (id,points) VALUES (?,?)");
                stm2.setLong(1, userId);
                stm2.setInt(2, cantidad);
                stm2.executeUpdate();
            }
        }
    }catch (SQLException exception){
        System.out.println("error recording points");
    }
}

 public ArrayList<CustomCommand> getCommands(long chatId) throws SQLException{
    ArrayList<CustomCommand> commands = new ArrayList<>();
    if(DatabaseConnector.getConnection() != null) {
        PreparedStatement stm = DatabaseConnector.getConnection().prepareStatement("SELECT text,fileID,commandText,type,probability FROM customcommands WHERE chatid = ?");
        stm.setLong(1, chatId);
        ResultSet results = stm.executeQuery();
        if(!results.isBeforeFirst()) return null;
        while (results.next()){
            commands.add(new CustomCommand(results.getString(1),results.getString(2),results.getString(3), CustomCommand.Type.valueOf(results.getString(4)),results.getInt(5)));
        }
        return commands;
    }
    return null;
}

Maybe the problem is something related to exception catching and statements not being correctly executed? Maybe something related to result sets? It's driving me crazy. Thanks for helping me!

Upvotes: 0

Views: 582

Answers (1)

duffymo
duffymo

Reputation: 308753

You do nothing to clean up ResultSet and Statement before you return. That's a bad idea. You should be closing each one in individual try/catch blocks in a finally block.

A ResultSet is an object that represents a database cursor on the database. You should close it so you don't run out of cursors.

I wouldn't have a single static Connection. I'd expect a thread-safe, managed pool of connections.

I wouldn't return a null. You don't make clear what the user is supposed to do with that. Better to throw an exception.

Upvotes: 2

Related Questions