Permalink
Browse files

Issue 145: cursor.execute leaks parameters

Thanks to luke (see issue) for finding, debugging, and providing a fix.
  • Loading branch information...
1 parent 791ba11 commit ce634d37e2d7a51a76859d0df2d166c1fd6081f2 Michael Kleehammer committed Sep 13, 2011
Showing with 16 additions and 3 deletions.
  1. +4 −0 src/cursor.h
  2. +12 −3 src/params.cpp
View
@@ -52,6 +52,10 @@ struct ParamInfo
// If true, the memory in ParameterValuePtr was allocated via malloc and must be freed.
bool allocated;
+ // The python object containing the parameter value. A reference to this
+ // object should be held until we have finished using memory owned by it.
+ PyObject *pyParameterValue;
+
// Optional data. If used, ParameterValuePtr will point into this.
union
{
View
@@ -20,8 +20,11 @@ static bool GetParamType(Cursor* cur, Py_ssize_t iParam, SQLSMALLINT& type);
static void FreeInfos(ParamInfo* a, Py_ssize_t count)
{
for (Py_ssize_t i = 0; i < count; i++)
+ {
if (a[i].allocated)
pyodbc_free(a[i].ParameterValuePtr);
+ Py_DECREF(a[i].pyParameterValue);
+ }
pyodbc_free(a);
}
@@ -165,7 +168,7 @@ static bool GetUnicodeInfo(Cursor* cur, Py_ssize_t index, PyObject* param, Param
info.ParameterValuePtr = pch;
}
#endif
-
+
info.ParameterType = SQL_WVARCHAR;
info.StrLen_or_Ind = (SQLINTEGER)(len * sizeof(SQLWCHAR));
}
@@ -454,6 +457,10 @@ static bool GetParameterInfo(Cursor* cur, Py_ssize_t index, PyObject* param, Par
{
// Binds the given parameter and populates `info`.
+ // Hold a reference to param until info is freed, because info will often be
+ // holding data borrowed from param.
+ info.pyParameterValue = param;
+
if (param == Py_None)
return GetNullInfo(cur, index, info);
@@ -560,8 +567,7 @@ bool PrepareAndBind(Cursor* cur, PyObject* pSql, PyObject* original_params, bool
//
// Since we may replace parameters (we replace objects with Py_True/Py_False when writing to a bit/bool column),
- // allocate an array and use it instead of the original sequence. Since we don't change ownership we don't bother
- // with incref. (That is, PySequence_GetItem will INCREF and ~ObjectArrayHolder will DECREF.)
+ // allocate an array and use it instead of the original sequence.
int params_offset = skip_first ? 1 : 0;
Py_ssize_t cParams = original_params == 0 ? 0 : PySequence_Length(original_params) - params_offset;
@@ -641,7 +647,10 @@ bool PrepareAndBind(Cursor* cur, PyObject* pSql, PyObject* original_params, bool
for (Py_ssize_t i = 0; i < cParams; i++)
{
+ // PySequence_GetItem returns a *new* reference
PyObject* param = PySequence_GetItem(original_params, i + params_offset);
+ // cur->paramInfos[i] assumes ownership of param reference here;
+ // it will be released when FreeInfos is called.
if (!GetParameterInfo(cur, i, param, cur->paramInfos[i]))
{
FreeInfos(cur->paramInfos, cParams);

0 comments on commit ce634d3

Please sign in to comment.