CODEWITHSUNDEEP
javaeclipsejdbcmemory-leaks
Chuchoo
Chuchoo

Reputation: 842

Resource Leak on PreparedStatement and ResultSet

Eclipse displays warning messages saying "Resource Leak, preparedstatement is not closed at following location"

To me it looks like finally block takes care of closing preparedstatement and rs object. Any idea why eclipse is still complaining on resource leak?

String ruleGrpId = null;
        int finalRuleID = 0;

        String saveRule1 = "INSERT INTO ..";
        String saveRule2 = "INSERT INTO ";

        String saveRule3 = "select ..._name = ?";

        PreparedStatement preparedstatement = null;
        ResultSet rs = null;

        try {
            if (objSomeRule.getRule() == 0) {

                preparedstatement = con.prepareStatement(saveRule1);
                preparedstatement.setString(1, ruleId);

                preparedstatement.executeUpdate();

                //Eclipse complains here
                preparedstatement = con.prepareStatement(saveRule2);

                preparedstatement.setInt(1, ruleID_2);
                preparedstatement.setString(2, ruleID_3);

                rs =  preparedstatement.executeQuery();

                if(rs.next()){
                    finalRuleID = rs.getInt(1);
                }else{
                    //complains here
                    preparedstatement = con.prepareStatement(saveRule3);
                    preparedstatement.setString(1, ruleID_4);
                    //complains here
                    rs =  preparedstatement.executeQuery();
                    if(rs.next()){
                        finalRuleID = rs.getInt("rulegroup_id");
                    }
                }
                objSomeRule.setRuleID(finalRuleID);

            }

        } finally {

            if(preparedstatement != null){
                preparedstatement.close();
            }

            if(rs != null){
                rs.close();
            }
        }

        return finalRuleID;
    }

Upvotes: 0

Views: 4568

Answers (3)

Thomas Fritsch
Thomas Fritsch

Reputation: 10127

You create 3 different instances of PreparedStatement:

preparedstatement = con.prepareStatement(saveRule1);

preparedstatement = con.prepareStatement(saveRule2);

preparedstatement = con.prepareStatement(saveRule3);

Your variable preparedStatement only holds the last one you opened, and you no have no references to the others you opened before.
Therefore, in your finally section you only close this one last PreparedStatement:

preparedstatement.close();

A similar issue is with your ResultSets. You create 2 instances, but you close only 1 of them.

Upvotes: 2

Joop Eggen
Joop Eggen

Reputation: 109547

The reuse of variables preparedStatement and rs prevents calling close() on the old object values of the variables. Try-with-resources is a neat solutions as close() is called automatically, even when an exception was thrown or a return executed in a deeply nested try.

    String ruleGrpId = null;
    int finalRuleID = 0;
    String saveRule1 = "INSERT INTO ..";
    String saveRule2 = "INSERT INTO ";
    String saveRule3 = "select ..._name = ?";

    if (objSomeRule.getRule() == 0) {
        try (PreparedStatement ps1 = con.prepareStatement(saveRule1)) {
            ps1.setString(1, ruleId);
            ps1.executeUpdate();
        }
        try (PreparedStatement ps2 = con.prepareStatement(saveRule2)) {
            ps2.setInt(1, ruleID_2);
            ps2.setString(2, ruleID_3);

            try (ResultSet rs2 =  ps2.executeQuery()) {

                if (rs2.next()) {
                    finalRuleID = rs2.getInt(1);
                } else {
                    try (Preparedstatement ps3 = con.prepareStatement(saveRule3)) {
                        ps3.setString(1, ruleID_4);
                        //complains here
                        try (ResultSet rs3 =  ps3.executeQuery()) {
                            if (rs3.next()) {
                                finalRuleID = rs3.getInt("rulegroup_id");
                            }
                        }
                    }
                }
            }
            objSomeRule.setRuleID(finalRuleID);
        }
    }

Should the last SELECT query intend to retrieve the AUTOINCREMENT ID of a prior INSERT, then this can be done safer (multi-user) and better using getGeneratedKeys

Upvotes: 3

Ravi
Ravi

Reputation: 31397

So, you are saying when there are no record create another preparedStatement.

 if(rs.next())
 {
      finalRuleID = rs.getInt(1);
 }
 else
 {
    //close here first
    if(preparedstatement != null)
    {
       preparedstatement.close();
    }

    if(rs != null){
       rs.close();
    }

    preparedstatement = con.prepareStatement(saveRule3);
    // your code

But, you haven't closed the previous one. I'm not exactly sure, why are you doing that, but, assuming you need to do this. Then, you should close the previous one first.

Upvotes: 2

Related Questions