From dc5af70631c86723132518152ce5f910848d83ae Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sun, 27 Jun 2004 23:32:34 +0000 Subject: 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. --- Lib/bsddb/test/test_recno.py | 9 ++++ Modules/_bsddb.c | 119 +++++++++++++++++++++++++++---------------- 2 files changed, 85 insertions(+), 43 deletions(-) diff --git a/Lib/bsddb/test/test_recno.py b/Lib/bsddb/test/test_recno.py index 87446d3..56a79c7 100644 --- a/Lib/bsddb/test/test_recno.py +++ b/Lib/bsddb/test/test_recno.py @@ -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() diff --git a/Modules/_bsddb.c b/Modules/_bsddb.c index 6eb4866..3ea2c5f 100644 --- a/Modules/_bsddb.c +++ b/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; } -- cgit v0.12