Bugfix for issue3885 and 'DB.verify()' crash.

Reviewed by Nick Coghlan.

Lib/bsddb/test/test_basics.py

@@ -573,6 +573,15 @@ def test06_Truncate(self):
 
     #----------------------------------------
 
+    def test07_verify(self):
+        # Verify bug solved in 4.7.3pre8
+        self.d.close()
+        d = db.DB(self.env)
+        d.verify(self.filename)
+
+
+    #----------------------------------------
+
 
 #----------------------------------------------------------------------
 
@@ -602,13 +611,13 @@ class BasicWithEnvTestCase(BasicTestCase):
 
     #----------------------------------------
 
-    def test07_EnvRemoveAndRename(self):
+    def test08_EnvRemoveAndRename(self):
         if not self.env:
             return
 
         if verbose:
             print '\n', '-=' * 30
-            print "Running %s.test07_EnvRemoveAndRename..." % self.__class__.__name__
+            print "Running %s.test08_EnvRemoveAndRename..." % self.__class__.__name__
 
         # can't rename or remove an open DB
         self.d.close()
@@ -619,7 +628,7 @@ def test07_EnvRemoveAndRename(self):
 
     # dbremove and dbrename are in 4.1 and later
     if db.version() < (4,1):
-        del test07_EnvRemoveAndRename
+        del test08_EnvRemoveAndRename
 
     #----------------------------------------
 
@@ -720,11 +729,11 @@ def test06_Transactions(self):
 
     #----------------------------------------
 
-    def test07_TxnTruncate(self):
+    def test08_TxnTruncate(self):
         d = self.d
         if verbose:
             print '\n', '-=' * 30
-            print "Running %s.test07_TxnTruncate..." % self.__class__.__name__
+            print "Running %s.test08_TxnTruncate..." % self.__class__.__name__
 
         d.put("abcde", "ABCDE");
         txn = self.env.txn_begin()
@@ -737,7 +746,7 @@ def test07_TxnTruncate(self):
 
     #----------------------------------------
 
-    def test08_TxnLateUse(self):
+    def test09_TxnLateUse(self):
         txn = self.env.txn_begin()
         txn.abort()
         try:
@@ -771,11 +780,11 @@ class BTreeRecnoTestCase(BasicTestCase):
     dbtype     = db.DB_BTREE
     dbsetflags = db.DB_RECNUM
 
-    def test07_RecnoInBTree(self):
+    def test08_RecnoInBTree(self):
         d = self.d
         if verbose:
             print '\n', '-=' * 30
-            print "Running %s.test07_RecnoInBTree..." % self.__class__.__name__
+            print "Running %s.test08_RecnoInBTree..." % self.__class__.__name__
 
         rec = d.get(200)
         self.assertEqual(type(rec), type(()))
@@ -805,11 +814,11 @@ class BTreeRecnoWithThreadFlagTestCase(BTreeRecnoTestCase):
 class BasicDUPTestCase(BasicTestCase):
     dbsetflags = db.DB_DUP
 
-    def test08_DuplicateKeys(self):
+    def test09_DuplicateKeys(self):
         d = self.d
         if verbose:
             print '\n', '-=' * 30
-            print "Running %s.test08_DuplicateKeys..." % \
+            print "Running %s.test09_DuplicateKeys..." % \
                   self.__class__.__name__
 
         d.put("dup0", "before")
@@ -878,11 +887,11 @@ def otherType(self):
         else:
             return db.DB_BTREE
 
-    def test09_MultiDB(self):
+    def test10_MultiDB(self):
         d1 = self.d
         if verbose:
             print '\n', '-=' * 30
-            print "Running %s.test09_MultiDB..." % self.__class__.__name__
+            print "Running %s.test10_MultiDB..." % self.__class__.__name__
 
         d2 = db.DB(self.env)
         d2.open(self.filename, "second", self.dbtype,
@@ -1014,9 +1023,20 @@ def setUp(self) :
         self.obj = db.DB()
 
 class CrashAndBurn(unittest.TestCase) :
-    def test01_OpenCrash(self) :
+    import sys
-        # See http://bugs.python.org/issue3307
+    if sys.version_info[:3] < (2, 4, 0):
-        self.assertRaises(db.DBInvalidArgError, db.DB, None, 65535)
+        def assertTrue(self, expr, msg=None):
+            self.failUnless(expr,msg=msg)
+
+    #def test01_OpenCrash(self) :
+    #    # See http://bugs.python.org/issue3307
+    #    self.assertRaises(db.DBInvalidArgError, db.DB, None, 65535)
+
+    def test02_DBEnv_dealloc(self):
+        # http://bugs.python.org/issue3885
+        import gc
+        self.assertRaises(db.DBInvalidArgError, db.DBEnv, ~db.DB_RPCCLIENT)
+        gc.collect()
 
 
 #----------------------------------------------------------------------
@@ -1044,7 +1064,7 @@ def test_suite():
     suite.addTest(unittest.makeSuite(HashMultiDBTestCase))
     suite.addTest(unittest.makeSuite(DBEnvPrivateObject))
     suite.addTest(unittest.makeSuite(DBPrivateObject))
-    #suite.addTest(unittest.makeSuite(CrashAndBurn))
+    suite.addTest(unittest.makeSuite(CrashAndBurn))
 
     return suite
 

Modules/_bsddb.c

@@ -989,7 +989,7 @@ newDBObject(DBEnvObject* arg, int flags)
 
 
 /* Forward declaration */
-static PyObject *DB_close_internal(DBObject* self, int flags);
+static PyObject *DB_close_internal(DBObject* self, int flags, int do_not_close);
 
 static void
 DB_dealloc(DBObject* self)
@@ -997,8 +997,15 @@ DB_dealloc(DBObject* self)
   PyObject *dummy;
 
     if (self->db != NULL) {
-      dummy=DB_close_internal(self,0);
+        dummy=DB_close_internal(self, 0, 0);
-      Py_XDECREF(dummy);
+        /*
+        ** Raising exceptions while doing
+        ** garbage collection is a fatal error.
+        */
+        if (dummy)
+            Py_DECREF(dummy);
+        else
+            PyErr_Clear();
     }
     if (self->in_weakreflist != NULL) {
         PyObject_ClearWeakRefs((PyObject *) self);
@@ -1052,8 +1059,15 @@ DBCursor_dealloc(DBCursorObject* self)
     PyObject *dummy;
 
     if (self->dbc != NULL) {
-      dummy=DBC_close_internal(self);
+        dummy=DBC_close_internal(self);
-      Py_XDECREF(dummy);
+        /*
+        ** Raising exceptions while doing
+        ** garbage collection is a fatal error.
+        */
+        if (dummy)
+            Py_DECREF(dummy);
+        else
+            PyErr_Clear();
     }
     if (self->in_weakreflist != NULL) {
         PyObject_ClearWeakRefs((PyObject *) self);
@@ -1071,6 +1085,7 @@ newDBEnvObject(int flags)
     if (self == NULL)
         return NULL;
 
+    self->db_env = NULL;
     self->closed = 1;
     self->flags = flags;
     self->moduleFlags.getReturnsNone = DEFAULT_GET_RETURNS_NONE;
@@ -1107,8 +1122,15 @@ DBEnv_dealloc(DBEnvObject* self)
   PyObject *dummy;
 
     if (self->db_env) {
-      dummy=DBEnv_close_internal(self,0);
+        dummy=DBEnv_close_internal(self, 0);
-      Py_XDECREF(dummy);
+        /*
+        ** Raising exceptions while doing
+        ** garbage collection is a fatal error.
+        */
+        if (dummy)
+            Py_DECREF(dummy);
+        else
+            PyErr_Clear();
     }
 
     Py_XDECREF(self->event_notifyCallback);
@@ -1186,8 +1208,17 @@ DBTxn_dealloc(DBTxnObject* self)
 
     if (self->txn) {
         int flag_prepare = self->flag_prepare;
+
         dummy=DBTxn_abort_discard_internal(self,0);
-        Py_XDECREF(dummy);
+        /*
+        ** Raising exceptions while doing
+        ** garbage collection is a fatal error.
+        */
+        if (dummy)
+            Py_DECREF(dummy);
+        else
+            PyErr_Clear();
+
         if (!flag_prepare) {
             PyErr_Warn(PyExc_RuntimeWarning,
               "DBTxn aborted in destructor.  No prior commit() or abort().");
@@ -1280,7 +1311,14 @@ DBSequence_dealloc(DBSequenceObject* self)
 
     if (self->sequence != NULL) {
         dummy=DBSequence_close_internal(self,0,0);
-        Py_XDECREF(dummy);
+        /*
+        ** Raising exceptions while doing
+        ** garbage collection is a fatal error.
+        */
+        if (dummy)
+            Py_DECREF(dummy);
+        else
+            PyErr_Clear();
     }
 
     if (self->in_weakreflist != NULL) {
@@ -1485,10 +1523,10 @@ DB_associate(DBObject* self, PyObject* args, PyObject* kwargs)
 
 
 static PyObject*
-DB_close_internal(DBObject* self, int flags)
+DB_close_internal(DBObject* self, int flags, int do_not_close)
 {
     PyObject *dummy;
-    int err;
+    int err = 0;
 
     if (self->db != NULL) {
         /* Can be NULL if db is not in an environment */
@@ -1511,10 +1549,20 @@ DB_close_internal(DBObject* self, int flags)
         }
 #endif
 
-        MYDB_BEGIN_ALLOW_THREADS;
+        /*
-        err = self->db->close(self->db, flags);
+        ** "do_not_close" is used to dispose all related objects in the
-        MYDB_END_ALLOW_THREADS;
+        ** tree, without actually releasing the "root" object.
-        self->db = NULL;
+        ** This is done, for example, because function calls like
+        ** "DB.verify()" implicitly close the underlying handle. So
+        ** the handle doesn't need to be closed, but related objects
+        ** must be cleaned up.
+        */
+        if (!do_not_close) {
+            MYDB_BEGIN_ALLOW_THREADS;
+            err = self->db->close(self->db, flags);
+            MYDB_END_ALLOW_THREADS;
+            self->db = NULL;
+        }
         RETURN_IF_ERR();
     }
     RETURN_NONE();
@@ -1526,7 +1574,7 @@ DB_close(DBObject* self, PyObject* args)
     int flags=0;
     if (!PyArg_ParseTuple(args,"|i:close", &flags))
         return NULL;
-    return DB_close_internal(self,flags);
+    return DB_close_internal(self, flags, 0);
 }
 
 
@@ -2146,7 +2194,7 @@ DB_open(DBObject* self, PyObject* args, PyObject* kwargs)
     if (makeDBError(err)) {
         PyObject *dummy;
 
-        dummy=DB_close_internal(self,0);
+        dummy=DB_close_internal(self, 0, 0);
         Py_XDECREF(dummy);
         return NULL;
     }
@@ -2840,21 +2888,24 @@ DB_verify(DBObject* self, PyObject* args, PyObject* kwargs)
 	/* XXX(nnorwitz): it should probably be an exception if outFile
 	   can't be opened. */
 
-    MYDB_BEGIN_ALLOW_THREADS;
-    err = self->db->verify(self->db, fileName, dbName, outFile, flags);
-    MYDB_END_ALLOW_THREADS;
-    if (outFile)
-        fclose(outFile);
-
     {  /* DB.verify acts as a DB handle destructor (like close) */
         PyObject *error;
 
-        error=DB_close_internal(self,0);
+        error=DB_close_internal(self, 0, 1);
         if (error ) {
           return error;
         }
      }
 
+    MYDB_BEGIN_ALLOW_THREADS;
+    err = self->db->verify(self->db, fileName, dbName, outFile, flags);
+    MYDB_END_ALLOW_THREADS;
+
+    self->db = NULL;  /* Implicit close; related objects already released */
+
+    if (outFile)
+        fclose(outFile);
+
     RETURN_IF_ERR();
     RETURN_NONE();
 }
@@ -3978,7 +4029,7 @@ DBEnv_close_internal(DBEnvObject* self, int flags)
           Py_XDECREF(dummy);
         }
         while(self->children_dbs) {
-          dummy=DB_close_internal(self->children_dbs,0);
+          dummy=DB_close_internal(self->children_dbs, 0, 0);
           Py_XDECREF(dummy);
         }
     }
@@ -4003,7 +4054,7 @@ DBEnv_close(DBEnvObject* self, PyObject* args)
 
     if (!PyArg_ParseTuple(args, "|i:close", &flags))
         return NULL;
-    return DBEnv_close_internal(self,flags);
+    return DBEnv_close_internal(self, flags);
 }
 
 
@@ -5949,7 +6000,7 @@ DBTxn_abort_discard_internal(DBTxnObject* self, int discard)
     }
 #endif
     while (self->children_dbs) {
-        dummy=DB_close_internal(self->children_dbs,0);
+        dummy=DB_close_internal(self->children_dbs, 0, 0);
         Py_XDECREF(dummy);
     }
 
@@ -6030,6 +6081,14 @@ DBSequence_close_internal(DBSequenceObject* self, int flags, int do_not_close)
             self->txn=NULL;
         }
 
+        /*
+        ** "do_not_close" is used to dispose all related objects in the
+        ** tree, without actually releasing the "root" object.
+        ** This is done, for example, because function calls like
+        ** "DBSequence.remove()" implicitly close the underlying handle. So
+        ** the handle doesn't need to be closed, but related objects
+        ** must be cleaned up.
+        */
         if (!do_not_close) {
             MYDB_BEGIN_ALLOW_THREADS
             err = self->sequence->close(self->sequence, flags);

Modules/bsddb.h

@@ -105,7 +105,7 @@
 #error "eek! DBVER can't handle minor versions > 9"
 #endif
 
-#define PY_BSDDB_VERSION "4.7.3pre5"
+#define PY_BSDDB_VERSION "4.7.3pre9"
 
 /* Python object definitions */