-
Notifications
You must be signed in to change notification settings - Fork 424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes all statement leaks in the driver. #455
Changes from 5 commits
35cfed9
d068bda
195719e
332a942
9859067
94f8829
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -563,6 +563,8 @@ private void checkClosed() throws SQLServerException { | |
assert null != st; | ||
stmtParent = st; | ||
con = st.connection; | ||
SQLServerStatement s = null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i guess s doesnt need to be instantiated to null but i did it for consistency |
||
SQLServerStatement stmt = null; | ||
if (logger.isLoggable(java.util.logging.Level.FINE)) { | ||
logger.fine(toString() + " created by (" + st.toString() + ")"); | ||
} | ||
|
@@ -571,7 +573,7 @@ private void checkClosed() throws SQLServerException { | |
// If the CallableStatement/PreparedStatement is a stored procedure call | ||
// then we can extract metadata using sp_sproc_columns | ||
if (null != st.procedureName) { | ||
SQLServerStatement s = (SQLServerStatement) con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); | ||
s = (SQLServerStatement) con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); | ||
String sProc = parseProcIdentifier(st.procedureName); | ||
if (con.isKatmaiOrLater()) | ||
rsProcedureMeta = s.executeQueryInternal("exec sp_sproc_columns_100 " + sProc + ", @ODBCVer=3"); | ||
|
@@ -659,13 +661,11 @@ private void checkClosed() throws SQLServerException { | |
|
||
String tablesAndJoins = sbTablesAndJoins.toString(); | ||
|
||
Statement stmt = con.createStatement(); | ||
stmt = (SQLServerStatement) con.createStatement(); | ||
String sCom = "sp_executesql N'SET FMTONLY ON SELECT " + columns + " FROM " + tablesAndJoins + " '"; | ||
|
||
ResultSet rs = stmt.executeQuery(sCom); | ||
parseQueryMetaFor2008(rs); | ||
stmt.close(); | ||
rs.close(); | ||
} | ||
} | ||
} | ||
|
@@ -679,6 +679,10 @@ private void checkClosed() throws SQLServerException { | |
catch(StringIndexOutOfBoundsException e){ | ||
SQLServerException.makeFromDriverError(con, stmtParent, e.toString(), null, false); | ||
} | ||
finally { | ||
if (null != stmt) | ||
stmt.close(); | ||
} | ||
} | ||
|
||
public boolean isWrapperFor(Class<?> iface) throws SQLException { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,6 +98,9 @@ public class SQLServerPreparedStatement extends SQLServerStatement implements IS | |
|
||
/** The prepared statement handle returned by the server */ | ||
private int prepStmtHandle = 0; | ||
|
||
/** Statement used for getMetadata(). Declared as a field to facilitate closing the statement. */ | ||
private SQLServerStatement internalStmt = null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. having internalStmt as a field is the only way to close it after the user is done working with the ResultsetMetadata. |
||
|
||
private void setPreparedStatementHandle(int handle) { | ||
this.prepStmtHandle = handle; | ||
|
@@ -271,6 +274,18 @@ final void closeInternal() { | |
|
||
// If we have a prepared statement handle, close it. | ||
closePreparedHandle(); | ||
|
||
// Close the statement that was used to generate empty statement from getMetadata(). | ||
try { | ||
if (null != internalStmt) | ||
internalStmt.close(); | ||
} catch (SQLServerException e) { | ||
if (loggerExternal.isLoggable(java.util.logging.Level.FINER)) | ||
loggerExternal.finer("Ignored error closing internal statement: " + e.getErrorCode() + " " + e.getMessage()); | ||
} | ||
finally { | ||
internalStmt = null; | ||
} | ||
|
||
// Clean up client-side state | ||
batchParamValues = null; | ||
|
@@ -1023,8 +1038,8 @@ else if (resultSet != null) { | |
ResultSet emptyResultSet = null; | ||
try { | ||
fmtSQL = replaceMarkerWithNull(fmtSQL); | ||
SQLServerStatement stmt = (SQLServerStatement) connection.createStatement(); | ||
emptyResultSet = stmt.executeQueryInternal("set fmtonly on " + fmtSQL + "\nset fmtonly off"); | ||
internalStmt = (SQLServerStatement) connection.createStatement(); | ||
emptyResultSet = internalStmt.executeQueryInternal("set fmtonly on " + fmtSQL + "\nset fmtonly off"); | ||
} | ||
catch (SQLException sqle) { | ||
if (false == sqle.getMessage().equals(SQLServerException.getErrString("R_noResultset"))) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we always omit the bracket when we can, so i did the same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually we started adding back the brackets even with one line of code, so lets keep the brackets here 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.