mirror of https://github.com/python/cpython
SF patch / bug #967763
Fix memory leaks revealed by valgrind and ensuing code inspection. In the existing test suite valgrind revealed two memory leaks (DB_get and DBC_set_range). Code inspection revealed that there were many other potential similar leaks (many on odd code error paths such as passing something other than a DBTxn object for a txn= parameter or in the face of an out of memory error). The most common case that would cause a leak was when using recno or queue format databases with integer keys, sometimes only with an exception exit.
This commit is contained in:
parent
c2b151c66e
commit
dc5af70631
|
@ -133,6 +133,13 @@ class SimpleRecnoTestCase(unittest.TestCase):
|
|||
if verbose:
|
||||
print rec
|
||||
|
||||
# test that non-existant key lookups work (and that
|
||||
# DBC_set_range doesn't have a memleak under valgrind)
|
||||
rec = c.set_range(999999)
|
||||
assert rec == None
|
||||
if verbose:
|
||||
print rec
|
||||
|
||||
c.close()
|
||||
d.close()
|
||||
|
||||
|
@ -177,6 +184,8 @@ class SimpleRecnoTestCase(unittest.TestCase):
|
|||
"""
|
||||
source = os.path.join(os.path.dirname(sys.argv[0]),
|
||||
'db_home/test_recno.txt')
|
||||
if not os.path.isdir('db_home'):
|
||||
os.mkdir('db_home')
|
||||
f = open(source, 'w') # create the file
|
||||
f.close()
|
||||
|
||||
|
|
119
Modules/_bsddb.c
119
Modules/_bsddb.c
|
@ -297,7 +297,7 @@ staticforward PyTypeObject DB_Type, DBCursor_Type, DBEnv_Type, DBTxn_Type, DBLoc
|
|||
#define CLEAR_DBT(dbt) (memset(&(dbt), 0, sizeof(dbt)))
|
||||
|
||||
#define FREE_DBT(dbt) if ((dbt.flags & (DB_DBT_MALLOC|DB_DBT_REALLOC)) && \
|
||||
dbt.data != NULL) { free(dbt.data); }
|
||||
dbt.data != NULL) { free(dbt.data); dbt.data = NULL; }
|
||||
|
||||
|
||||
static int makeDBError(int err);
|
||||
|
@ -330,7 +330,7 @@ static int make_dbt(PyObject* obj, DBT* dbt)
|
|||
}
|
||||
else if (!PyArg_Parse(obj, "s#", &dbt->data, &dbt->size)) {
|
||||
PyErr_SetString(PyExc_TypeError,
|
||||
"Key and Data values must be of type string or None.");
|
||||
"Data values must be of type string or None.");
|
||||
return 0;
|
||||
}
|
||||
return 1;
|
||||
|
@ -340,7 +340,7 @@ static int make_dbt(PyObject* obj, DBT* dbt)
|
|||
/* Recno and Queue DBs can have integer keys. This function figures out
|
||||
what's been given, verifies that it's allowed, and then makes the DBT.
|
||||
|
||||
Caller should call FREE_DBT(key) when done. */
|
||||
Caller MUST call FREE_DBT(key) when done. */
|
||||
static int
|
||||
make_key_dbt(DBObject* self, PyObject* keyobj, DBT* key, int* pflags)
|
||||
{
|
||||
|
@ -1298,11 +1298,15 @@ DB_delete(DBObject* self, PyObject* args, PyObject* kwargs)
|
|||
CHECK_DB_NOT_CLOSED(self);
|
||||
if (!make_key_dbt(self, keyobj, &key, NULL))
|
||||
return NULL;
|
||||
if (!checkTxnObj(txnobj, &txn))
|
||||
if (!checkTxnObj(txnobj, &txn)) {
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (-1 == _DB_delete(self, txn, &key, 0))
|
||||
if (-1 == _DB_delete(self, txn, &key, 0)) {
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
FREE_DBT(key);
|
||||
RETURN_NONE();
|
||||
|
@ -1348,16 +1352,20 @@ DB_get(DBObject* self, PyObject* args, PyObject* kwargs)
|
|||
CHECK_DB_NOT_CLOSED(self);
|
||||
if (!make_key_dbt(self, keyobj, &key, &flags))
|
||||
return NULL;
|
||||
if (!checkTxnObj(txnobj, &txn))
|
||||
if (!checkTxnObj(txnobj, &txn)) {
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
CLEAR_DBT(data);
|
||||
if (CHECK_DBFLAG(self, DB_THREAD)) {
|
||||
/* Tell BerkeleyDB to malloc the return value (thread safe) */
|
||||
data.flags = DB_DBT_MALLOC;
|
||||
}
|
||||
if (!add_partial_dbt(&data, dlen, doff))
|
||||
if (!add_partial_dbt(&data, dlen, doff)) {
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
MYDB_BEGIN_ALLOW_THREADS;
|
||||
err = self->db->get(self->db, txn, &key, &data, flags);
|
||||
|
@ -1379,9 +1387,9 @@ DB_get(DBObject* self, PyObject* args, PyObject* kwargs)
|
|||
data.size);
|
||||
else /* return just the data */
|
||||
retval = PyString_FromStringAndSize((char*)data.data, data.size);
|
||||
FREE_DBT(key);
|
||||
FREE_DBT(data);
|
||||
}
|
||||
FREE_DBT(key);
|
||||
|
||||
RETURN_IF_ERR();
|
||||
return retval;
|
||||
|
@ -1406,8 +1414,10 @@ DB_get_size(DBObject* self, PyObject* args, PyObject* kwargs)
|
|||
CHECK_DB_NOT_CLOSED(self);
|
||||
if (!make_key_dbt(self, keyobj, &key, &flags))
|
||||
return NULL;
|
||||
if (!checkTxnObj(txnobj, &txn))
|
||||
if (!checkTxnObj(txnobj, &txn)) {
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
CLEAR_DBT(data);
|
||||
|
||||
/* We don't allocate any memory, forcing a ENOMEM error and thus
|
||||
|
@ -1449,10 +1459,12 @@ DB_get_both(DBObject* self, PyObject* args, PyObject* kwargs)
|
|||
CHECK_DB_NOT_CLOSED(self);
|
||||
if (!make_key_dbt(self, keyobj, &key, NULL))
|
||||
return NULL;
|
||||
if (!make_dbt(dataobj, &data))
|
||||
return NULL;
|
||||
if (!checkTxnObj(txnobj, &txn))
|
||||
if ( !make_dbt(dataobj, &data) ||
|
||||
!checkTxnObj(txnobj, &txn) )
|
||||
{
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
flags |= DB_GET_BOTH;
|
||||
|
||||
|
@ -1719,10 +1731,15 @@ DB_put(DBObject* self, PyObject* args, PyObject* kwargs)
|
|||
return NULL;
|
||||
|
||||
CHECK_DB_NOT_CLOSED(self);
|
||||
if (!make_key_dbt(self, keyobj, &key, NULL)) return NULL;
|
||||
if (!make_dbt(dataobj, &data)) return NULL;
|
||||
if (!add_partial_dbt(&data, dlen, doff)) return NULL;
|
||||
if (!checkTxnObj(txnobj, &txn)) return NULL;
|
||||
if (!make_key_dbt(self, keyobj, &key, NULL))
|
||||
return NULL;
|
||||
if ( !make_dbt(dataobj, &data) ||
|
||||
!add_partial_dbt(&data, dlen, doff) ||
|
||||
!checkTxnObj(txnobj, &txn) )
|
||||
{
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (-1 == _DB_put(self, txn, &key, &data, flags)) {
|
||||
FREE_DBT(key);
|
||||
|
@ -2390,8 +2407,10 @@ DB_has_key(DBObject* self, PyObject* args)
|
|||
CHECK_DB_NOT_CLOSED(self);
|
||||
if (!make_key_dbt(self, keyobj, &key, NULL))
|
||||
return NULL;
|
||||
if (!checkTxnObj(txnobj, &txn))
|
||||
if (!checkTxnObj(txnobj, &txn)) {
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
/* This causes ENOMEM to be returned when the db has the key because
|
||||
it has a record but can't allocate a buffer for the data. This saves
|
||||
|
@ -2692,21 +2711,24 @@ DBC_get(DBCursorObject* self, PyObject* args, PyObject *kwargs)
|
|||
|
||||
if (keyobj && !make_key_dbt(self->mydb, keyobj, &key, NULL))
|
||||
return NULL;
|
||||
if (dataobj && !make_dbt(dataobj, &data))
|
||||
return NULL;
|
||||
if (!add_partial_dbt(&data, dlen, doff))
|
||||
if ( (dataobj && !make_dbt(dataobj, &data)) ||
|
||||
(!add_partial_dbt(&data, dlen, doff)) )
|
||||
{
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (CHECK_DBFLAG(self->mydb, DB_THREAD)) {
|
||||
data.flags = DB_DBT_MALLOC;
|
||||
key.flags = DB_DBT_MALLOC;
|
||||
if (!(key.flags & DB_DBT_REALLOC)) {
|
||||
key.flags |= DB_DBT_MALLOC;
|
||||
}
|
||||
}
|
||||
|
||||
MYDB_BEGIN_ALLOW_THREADS;
|
||||
err = self->dbc->c_get(self->dbc, &key, &data, flags);
|
||||
MYDB_END_ALLOW_THREADS;
|
||||
|
||||
|
||||
if ((err == DB_NOTFOUND) && self->mydb->moduleFlags.getReturnsNone) {
|
||||
Py_INCREF(Py_None);
|
||||
retval = Py_None;
|
||||
|
@ -2731,9 +2753,9 @@ DBC_get(DBCursorObject* self, PyObject* args, PyObject *kwargs)
|
|||
data.data, data.size);
|
||||
break;
|
||||
}
|
||||
FREE_DBT(key);
|
||||
FREE_DBT(data);
|
||||
}
|
||||
FREE_DBT(key);
|
||||
return retval;
|
||||
}
|
||||
|
||||
|
@ -2810,9 +2832,12 @@ DBC_put(DBCursorObject* self, PyObject* args, PyObject* kwargs)
|
|||
|
||||
if (!make_key_dbt(self->mydb, keyobj, &key, NULL))
|
||||
return NULL;
|
||||
if (!make_dbt(dataobj, &data))
|
||||
if (!make_dbt(dataobj, &data) ||
|
||||
!add_partial_dbt(&data, dlen, doff) )
|
||||
{
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
if (!add_partial_dbt(&data, dlen, doff)) return NULL;
|
||||
}
|
||||
|
||||
MYDB_BEGIN_ALLOW_THREADS;
|
||||
err = self->dbc->c_put(self->dbc, &key, &data, flags);
|
||||
|
@ -2848,8 +2873,10 @@ DBC_set(DBCursorObject* self, PyObject* args, PyObject *kwargs)
|
|||
/* Tell BerkeleyDB to malloc the return value (thread safe) */
|
||||
data.flags = DB_DBT_MALLOC;
|
||||
}
|
||||
if (!add_partial_dbt(&data, dlen, doff))
|
||||
if (!add_partial_dbt(&data, dlen, doff)) {
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
MYDB_BEGIN_ALLOW_THREADS;
|
||||
err = self->dbc->c_get(self->dbc, &key, &data, flags|DB_SET);
|
||||
|
@ -2878,9 +2905,9 @@ DBC_set(DBCursorObject* self, PyObject* args, PyObject *kwargs)
|
|||
data.data, data.size);
|
||||
break;
|
||||
}
|
||||
FREE_DBT(key);
|
||||
FREE_DBT(data);
|
||||
}
|
||||
FREE_DBT(key);
|
||||
|
||||
return retval;
|
||||
}
|
||||
|
@ -2906,13 +2933,18 @@ DBC_set_range(DBCursorObject* self, PyObject* args, PyObject* kwargs)
|
|||
return NULL;
|
||||
|
||||
CLEAR_DBT(data);
|
||||
if (!add_partial_dbt(&data, dlen, doff)) {
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
if (CHECK_DBFLAG(self->mydb, DB_THREAD)) {
|
||||
/* Tell BerkeleyDB to malloc the return value (thread safe) */
|
||||
data.flags = DB_DBT_MALLOC;
|
||||
key.flags = DB_DBT_MALLOC;
|
||||
data.flags |= DB_DBT_MALLOC;
|
||||
/* only BTREE databases will return anything in the key */
|
||||
if (!(key.flags & DB_DBT_REALLOC) && _DB_get_type(self->mydb) == DB_BTREE) {
|
||||
key.flags |= DB_DBT_MALLOC;
|
||||
}
|
||||
}
|
||||
if (!add_partial_dbt(&data, dlen, doff))
|
||||
return NULL;
|
||||
MYDB_BEGIN_ALLOW_THREADS;
|
||||
err = self->dbc->c_get(self->dbc, &key, &data, flags|DB_SET_RANGE);
|
||||
MYDB_END_ALLOW_THREADS;
|
||||
|
@ -2940,17 +2972,14 @@ DBC_set_range(DBCursorObject* self, PyObject* args, PyObject* kwargs)
|
|||
data.data, data.size);
|
||||
break;
|
||||
}
|
||||
if (_DB_get_type(self->mydb) == DB_BTREE) {
|
||||
/* the only time a malloced key is returned is when we
|
||||
* call this on a BTree database because it performs
|
||||
* partial matching and needs to return the real key.
|
||||
* All others leave key untouched [where calling free()
|
||||
* on it would often segfault].
|
||||
*/
|
||||
FREE_DBT(key);
|
||||
}
|
||||
FREE_DBT(key);
|
||||
FREE_DBT(data);
|
||||
}
|
||||
/* the only time REALLOC should be set is if we used an integer
|
||||
* key that make_dbt_key malloc'd for us. always free these. */
|
||||
if (key.flags & DB_DBT_REALLOC) {
|
||||
FREE_DBT(key);
|
||||
}
|
||||
|
||||
return retval;
|
||||
}
|
||||
|
@ -2966,8 +2995,10 @@ _DBC_get_set_both(DBCursorObject* self, PyObject* keyobj, PyObject* dataobj,
|
|||
/* the caller did this: CHECK_CURSOR_NOT_CLOSED(self); */
|
||||
if (!make_key_dbt(self->mydb, keyobj, &key, NULL))
|
||||
return NULL;
|
||||
if (!make_dbt(dataobj, &data))
|
||||
if (!make_dbt(dataobj, &data)) {
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
MYDB_BEGIN_ALLOW_THREADS;
|
||||
err = self->dbc->c_get(self->dbc, &key, &data, flags|DB_GET_BOTH);
|
||||
|
@ -3104,8 +3135,10 @@ DBC_set_recno(DBCursorObject* self, PyObject* args, PyObject *kwargs)
|
|||
/* Tell BerkeleyDB to malloc the return value (thread safe) */
|
||||
data.flags = DB_DBT_MALLOC;
|
||||
}
|
||||
if (!add_partial_dbt(&data, dlen, doff))
|
||||
if (!add_partial_dbt(&data, dlen, doff)) {
|
||||
FREE_DBT(key);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
MYDB_BEGIN_ALLOW_THREADS;
|
||||
err = self->dbc->c_get(self->dbc, &key, &data, flags|DB_SET_RECNO);
|
||||
|
@ -3120,9 +3153,9 @@ DBC_set_recno(DBCursorObject* self, PyObject* args, PyObject *kwargs)
|
|||
else { /* Can only be used for BTrees, so no need to return int key */
|
||||
retval = Py_BuildValue("s#s#", key.data, key.size,
|
||||
data.data, data.size);
|
||||
FREE_DBT(key);
|
||||
FREE_DBT(data);
|
||||
}
|
||||
FREE_DBT(key);
|
||||
|
||||
return retval;
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue