mysql memory leak on my code?

I'm really new to java, and I need to know is this a memory leak ?

This is a piece of code from a game server I'm trying to improve:

private static void closePreparedStatement(PreparedStatement p)
{
    try {
        p.clearParameters();
        p.close();
    } catch (SQLException e) {e.printStackTrace();}
}

public static void UPDATE_ACCOUNT(Compte acc)
{
    try
    {
        String baseQuery = "UPDATE accounts SET " +
                            "`bankGold` = ?,"+
                            "`bank` = ?,"+
                            "`level` = ?,"+
                            "`stable` = ?,"+
                            "`banned` = ?,"+
                            "`enemy` = ?"+
                            " WHERE `guid` = ?;";
        PreparedStatement p = newTransact(baseQuery, dConnexion);

        p.setLong(1, acc.getBankGold());
        p.setString(2, acc.parseBankObjectsToDB());
        p.setInt(3, acc.get_Lvl());
        p.setString(4, acc.parseStable());
        p.setInt(5, (acc.isBanned()?1:0));
        p.setString(6, ac开发者_高级运维c.parseEnemyListToDB());
        p.setInt(7, acc.getGUID());

        p.executeUpdate();
        closePreparedStatement(p);
    }catch(SQLException e)
    {
        LogClass.addErrorLog("MySQL ERROR: "+e.getMessage());
        e.printStackTrace();
    }
}

after much thinking and readings, I think maybe a finally catch block with a closePreparedStatement(p) inside of it would be a good idea to avoid memory leaks?

    finally
    {
        closePreparedStatement(p);
    }

If so, then I have a lot of work to do to change hundreds of methods in this way. I'm just asking your opinion guys before I regret it later since I'm a total newbie

Need an answer please, and if you have any opinion on this piece of code about memory leaks, please don't hesitate.

Thank you for your time.

You are right in your finally block assumptions. Say for example a connection is opened and then an error occurs in your code. Luckily (for some extent) you have a try catch block so you can catch and then do what you may with the error weather it be throw the error or display error or log the error or all of the above (there are many ways to deal with errors and every errors should be dealt with one way or another). Unfortunately if an error is caught in a catch block and a connection (perhaps to a database or file etc.. is still open that connections is tying up resources and impending the machine which the source is running on. Finally blocks to release this connections ARE KEY as your presumption predicts.

If there are multiple connections then the leak may be occuring elsewhere. For a rule of thumb it is good practice to use a finally block to close connections that have been opened in a try block. Happy Coding!

Given the code a final answer is not possible, e.g. how do you retrieve the connection and statement in newTransact. Where do you close the connection?

In general you're right that the final clause is missing, but you must be aware that in most of the cases your preparedstatement will already be closed, only when it comes to an exception it will be missed. To conclude: introducing the finally block is good, but I bet it will not solve your memory leak problem.

You have to trace your application to find out where the leak might be!

trying to close statement in finally block is definitely best practice.

You should really look at wikipedia about JDBC: http://en.wikipedia.org/wiki/Java_Database_Connectivity There are very nice examples how to connect to database in Java without causing a memory leak.

edit: If you really have to connect to database using JDBC, I would recomend using Spring - there's a great class JdbcTemplate, which hides lots of boilerplate code and you can focus on the important stuff (getting data from/to database) and not worry about memory leaks.

If an exception happens in your following statement:

PreparedStatement p = newTransact(baseQuery, dConnexion);

then p might be not initialized and when you try to close it in the finally block, it will generate an error. now even though you have a try catch in your method that closes the statement, this is not an error that is supposed to be logged as the original error was already dealt with. therefore, i believe it is a good practice in such methods to check that the statement is not null before trying to close it. so modifying your method to the following might be better:

    private static void closePreparedStatement(PreparedStatement p)
{
  if (p!=null){   
    try {
        p.clearParameters();
        p.close();
    } catch (SQLException e) {e.printStackTrace();}
  }
}