java - Database connection leak -
recently, had measure time taken of sql requests of our software. that, decided take naive approach , surround queries calls system.nanotime(). while doing found class (sqlsuggest) containing 4 similar queries in 4 similar methods. thought idea refactor , regroup common parts.
this created connection leak, connections weren't closed anymore. rolled refactor i'd still understand did wrong.
the first version of sqlsuggest class has 4 methods getsuggestlistbyitems, getdisplayvaluebyitems, getsuggestlistbylistid , getdisplayvaluebylisteid. each of these methods opens , closes connection, statement , resultset (in block) through class: dbaccess.
the second version of class has same 4 methods except instead of opening , closing connection, call 1 of 2 methods depending on whether need 1 result or list.
those 2 methods (executequerygetstring , executequerygetlistofstringarray) each declare connection, statement , resultset, call method: executequery connection open, statement created , resultset returned.
the second-tier method extracts data resultset , closes everything.
i'm guessing wrong in thinking declaring connection, statement , resultset in method allow me close them same method.
here old class (simplified):
public class sqlsuggest { private final static logger logger = logmanager.getlogger(); public list<string[]> getsuggestlistbyitems(string codesite) { list<string[]> suggestlist = new arraylist<>(); string query = "select idcode, designation mytable codesite = " + codesite; // simplified query building dbaccess accesbd = new dbaccess(); connection conn = null; statement stmt = null; resultset rs = null; try { conn = accesbd.getconnection(); stmt = conn.createstatement(); rs = stmt.executequery(query); while (rs.next()) { suggestlist.add(new string[]{rs.getstring(1), rs.getstring(2)}); } } catch (namingexception | sqlexception ex) { logger.error("", ex); } { accesbd.closeresultset(rs); accesbd.closestatement(stmt); accesbd.closeconnection(conn); } return suggestlist; } public string getdisplayvaluebyitems(string table, string codesite) { string displayvalue = null; string query = "select idcode, designation " + table + " codesite = " + codesite; // different query building dbaccess accesbd = new dbaccess(); connection conn = null; statement stmt = null; resultset rs = null; try { conn = accesbd.getconnection(); stmt = conn.createstatement(); rs = stmt.executequery(query); if (rs.next()) { displayvalue = rs.getstring(1); } } catch (namingexception | sqlexception ex) { logger.error("", ex); } { accesbd.closeresultset(rs); accesbd.closestatement(stmt); accesbd.closeconnection(conn); } return displayvalue; } public list<string[]> getsuggestlistbylistid(string listeid, string table, string codesite) { list<string[]> suggestlist = new arraylist<>(); string query = "select idcode, designation " + table + " codesite = " + codesite; // different query building involving listid dbaccess accesbd = new dbaccess(); connection conn = null; statement stmt = null; resultset rs = null; try { conn = accesbd.getconnection(); stmt = conn.createstatement(); rs = stmt.executequery(query); while (rs.next()) { suggestlist.add(new string[]{rs.getstring(1), rs.getstring(2)}); } } catch (namingexception | sqlexception ex) { logger.error("", ex); } { accesbd.closeresultset(rs); accesbd.closestatement(stmt); accesbd.closeconnection(conn); } return suggestlist; } public string getdisplayvaluebylisteid(string listeid, string table, string codesite) { string displayvalue = null; string query = "select idcode, designation " + table + " codesite = " + codesite; // different query building involving listid dbaccess accesbd = new dbaccess(); connection conn = null; statement stmt = null; resultset rs = null; try { conn = accesbd.getconnection(); stmt = conn.createstatement(); rs = stmt.executequery(query); if (rs.next()) { displayvalue = rs.getstring(2); } } catch (namingexception | sqlexception ex) { logger.error("", ex); } { accesbd.closeresultset(rs); accesbd.closestatement(stmt); accesbd.closeconnection(conn); } return displayvalue; } } here new class (the 1 leak):
public class sqlsuggest { private static final logger logger = logmanager.getlogger(); public list<string[]> getsuggestlistbyitems(string codesite) { list<string[]> suggestlist; string query = "select idcode, designation mytable codesite = " + codesite; // simplified query building suggestlist = executequerygetlistofstringarray(query); return suggestlist; } public string getdisplayvaluebyitems(string table, string codesite) { string displayvalue = null; string query = "select idcode, designation " + table + " codesite = " + codesite; // different query building int columntofetch = 1; displayvalue = executequerygetstring(query, columntofetch); return displayvalue; } public list<string[]> getsuggestlistbylistid(string listeid, string table, string codesite) { list<string[]> suggestlist = new arraylist<>(); string query = "select idcode, designation " + table + " codesite = " + codesite; // different query building involving listid suggestlist = executequerygetlistofstringarray(query); return suggestlist; } public string getdisplayvaluebylisteid(string listeid, string table, string codesite) { string displayvalue = null; string query = "select idcode, designation " + table + " codesite = " + codesite; // different query building involving listid int columntofetch = 2; displayvalue = executequerygetstring(query, columntofetch); return displayvalue; } private string executequerygetstring(string query, int columntofetch){ string result = null; statement stmt = null; connection conn = null; resultset rs = executequery(query, stmt, conn); try{ if (rs.next()) { result = rs.getstring(columntofetch); } } catch (sqlexception ex) { logger.error("", ex); } { dbaccess accesbd = new dbaccess(); accesbd.closeresultset(rs); accesbd.closestatement(stmt); accesbd.closeconnection(conn); } return result; } private list<string[]> executequerygetlistofstringarray(string query){ list<string[]> result = new arraylist<>(); statement stmt = null; connection conn = null; resultset rs = executequery(query, stmt, conn); try{ while (rs.next()) { result.add(new string[]{rs.getstring(1), rs.getstring(2)}); } } catch (sqlexception ex) { logger.error("", ex); } { dbaccess accesbd = new dbaccess(); accesbd.closeresultset(rs); accesbd.closestatement(stmt); accesbd.closeconnection(conn); } return result; } private resultset executequery(string query, statement stmt, connection conn){ dbaccess accesbd = new dbaccess(); resultset rs = null; try { conn = accesbd.getconnection(); stmt = conn.createstatement(); rs = stmt.executequery(query); } catch (namingexception | sqlexception ex) { logger.error("", ex); } return rs; } } and here class opens , closes connections (which wasn't changed in way):
public class dbaccess { private final static logger logger = logmanager.getlogger(); public void closeresultset(resultset rs) { if (rs != null) { try { rs.close(); } catch (exception e) { logger.error("", e); } } } public void closestatement(statement stmt) { if (stmt != null) { try { stmt.close(); } catch (exception e) { logger.error("", e); } } } public void closeconnection(connection conn) { if (conn != null) { try { conn.close(); } catch (exception e) { logger.error("", e); } } } public connection getconnection() throws sqlexception, namingexception { connection cnx = null; propertiesmanager manager = propertiesdelegate.getpropertiesmanager(); string jndi = manager.getproperty("datasource.name", "quartisweb-pu"); context ctx = null; datasource datasource = null; try { ctx = new initialcontext(); datasource = (datasource) ctx.lookup(jndi); } catch (namingexception ex) { try { datasource = (datasource) ctx.lookup("java:comp/env/" + jndi); } catch (namingexception ex1) { logger.error("", ex1); } } if (datasource != null) { cnx = datasource.getconnection(); } return cnx; } } please disregard fact parameters aren't correctly set in query, know should done.
when make call executequerygetstring() executequery() value of conn , stmt change inside executequery() remain null inside executequerygetstring() upon return. therefore, @ finally of executequerygetstring() call accesbd.closeconnection(conn) doing accesbd.closeconnection(null)
Comments
Post a Comment