Clean up the valgrind memory leaks (caused by lots of subtle c++ bugs. Addresse #215

git-svn-id: file:///svn/tokudb@1320 c7de825b-a66e-492c-adef-691d508d4ae1

cxx/Makefile

@@ -15,3 +15,7 @@ test1: test1.o dbt.o db.o dbenv.o ../lib/libdb.a
 
 $(LIBNAME).a: $(OBJS)
 	$(AR) rv $@ $(OBJS)
+clean:
+	rm -f $(OBJS) $(LIBNAME).a $(LIBNAME).so
+
+

cxx/db.cpp

@@ -27,11 +27,13 @@ Db::Db(DbEnv *env, u_int32_t flags)
 }
 
 Db::~Db() {
-    if (is_private_env) {
-	delete the_Env; // The destructor closes the env.
+    if (is_private_env && the_Env) {
+	the_Env->close(0);
+	delete the_Env;
     }
-    if (!the_db) {
+    if (the_db) {
 	close(0); // the user should have called close, but we do it here if not done.
+	assert(the_db==0);
     }
 }
 
@@ -41,13 +43,25 @@ int Db::close (u_int32_t flags) {
     }
     the_db->api_internal = 0;
 
+
     int ret = the_db->close(the_db, flags);
 
     the_db = 0;
+
+    int no_exceptions = the_Env->do_no_exceptions; // Get this out before possibly deleting the env
+    
+    if (is_private_env) {
+	// The env was closed by the_db->close, we have to tell the DbEnv that the DB_ENV is gone, and delete it.
+	the_Env->the_env = 0;
+	delete the_Env;
+	the_Env=0;
+    }
+
     // Do we need to clean up "private environments"?
     // What about cursors?  They should be cleaned up already, but who did it?
 
-    return the_Env->maybe_throw_error(ret);
+    // This maybe_throw must be the static one because the env is gone.
+    return DbEnv::maybe_throw_error(ret, NULL, no_exceptions);
 }
 
 int Db::open(DbTxn *txn, const char *filename, const char *subname, DBTYPE typ, u_int32_t flags, int mode) {
@@ -72,6 +86,7 @@ int Db::set_pagesize(u_int32_t size) {
 
 int Db::remove(const char *file, const char *database, u_int32_t flags) {
     int ret = the_db->remove(the_db, file, database, flags);
+    the_db = 0;
     return the_Env->maybe_throw_error(ret);
 }
 

cxx/dbc.cpp

@@ -2,8 +2,8 @@
 
 int Dbc::close (void) {
     DBC *dbc = this;
+    DbEnv *env = (DbEnv*)dbc->dbp->api_internal; // Must grab the env before closing the cursor.
     int ret = dbc->c_close(dbc);
-    DbEnv *env = (DbEnv*)dbc->dbp->api_internal;
     return env->maybe_throw_error(ret);
 }
 

cxx/dbenv.cpp

@@ -23,9 +23,18 @@ DbEnv::DbEnv(DB_ENV *env, u_int32_t flags)
     the_env->api1_internal = this;
 }
 
+// If still open, close it.  In most cases, the caller should call close explicitly so that they can catch the exceptions.
+DbEnv::~DbEnv(void)
+{
+    if (the_env!=NULL) {
+	(void)the_env->close(the_env, 0);
+	the_env = 0;
+    }
+}
+
 int DbEnv::close(u_int32_t flags) {
     int ret = the_env->close(the_env, flags);
-    the_env = 0; /* get rid of the env ref, so we don't touch it (even if we failed.) */
+    the_env = 0; /* get rid of the env ref, so we don't touch it (even if we failed, or when the destructor is called) */
     return maybe_throw_error(ret);
 }
 
@@ -64,19 +73,23 @@ void DbEnv::set_errpfx(const char *errpfx) {
     the_env->set_errpfx(the_env, errpfx);
 }
 
-int DbEnv::maybe_throw_error(int err) throw (DbException) {
+int DbEnv::maybe_throw_error(int err, DbEnv *env, int no_exceptions) throw (DbException) {
     if (err==0) return 0;
-    if (do_no_exceptions) return err;
+    if (no_exceptions) return err;
     if (err==DB_LOCK_DEADLOCK) {
-	DbDeadlockException e(this);
+	DbDeadlockException e(env);
 	throw e;
     } else {
 	DbException e(err);
-	e.set_env(this);
+	e.set_env(env);
 	throw e;
     }
 }
 
+int DbEnv::maybe_throw_error(int err) throw (DbException) {
+    return maybe_throw_error(err, this, do_no_exceptions);
+}
+
 extern "C" {
     void toku_db_env_err_vararg(const DB_ENV * env, int error, const char *fmt, va_list ap);
 };

cxx/tests/Makefile

@@ -5,6 +5,12 @@ CPPFLAGS = -I../ -I../../include
 CXXFLAGS = -Wall -g
 LDLIBS = ../../lib/libtdb_cxx.a ../../lib/libdb.a -lz
 
+ifeq ($(OSX),OSX)
+ VGRIND=
+else
+ VGRIND=valgrind --quiet --error-exitcode=1 --leak-check=yes
+endif
+
 all: $(TARGETS)
 $(TARGETS): $(DBCXX)
 
@@ -15,5 +21,9 @@ clean:
 	rm -rf $(TARGETS)
 
 check: $(TARGETS)
-	./test1
-	./test1e
+	$(VGRIND) ./test1
+	$(VGRIND) ./test1e
+	$(VGRIND) ./db_create foo.db a b c d
+	$(VGRIND) ./db_dump foo.db > foo.out
+	(echo " 61";echo " 62";echo " 63";echo " 64")>foo.expectout
+	diff foo.out foo.expectout

cxx/tests/test1.cpp

@@ -36,6 +36,8 @@ void test_db(void) {
     
     r = db.set_bt_compare(cmp);                 assert(r == 0);
     r = db.remove("DoesNotExist.db", NULL, 0);  assert(r == ENOENT);
+    // The db is closed
+    r = env.close(0);                           assert(r== 0);
 }
 
 void test_db_env(void) {
@@ -55,6 +57,5 @@ int main()
     test_dbt();
     test_db();
     test_db_env();
-    cout << "Hello World!" << endl;   cout << "Welcome to C++ Programming" << endl;
     return 0;
 }

cxx/tests/test1e.cpp

@@ -68,6 +68,5 @@ int main()
     test_dbt();
     test_db();
     test_db_env();
-    cout << "Hello World!" << endl;   cout << "Welcome to C++ Programming" << endl;
     return 0;
 }

cxx/txn.cpp

@@ -9,5 +9,6 @@ DbTxn::DbTxn(DB_TXN *txn)
 int DbTxn::commit (u_int32_t flags) {
     DB_TXN *txn = get_DB_TXN();
     int ret = txn->commit(txn, flags);
-    return ret;
+    DbEnv *env = (DbEnv*)txn->mgrp->api1_internal;
+    return env->maybe_throw_error(ret);
 }

include/db.h

@@ -211,8 +211,9 @@ struct __toku_db_txn_active {
   char __toku_dummy2[184];  /* Padding at the end */ 
 };
 struct __toku_db_txn {
+  DB_ENV *mgrp /*In TokuDB, mgrp is a DB_ENV not a DB_TXNMGR*/; /* 32-bit offset=0 size=4, 64=bit offset=0 size=8 */
   struct __toku_db_txn_internal *i;
-  void* __toku_dummy0[18];
+  void* __toku_dummy0[17];
   char __toku_dummy1[8];
   void *api_internal; /* 32-bit offset=84 size=4, 64=bit offset=160 size=8 */
   void* __toku_dummy2[2];

include/db_cxx.h

@@ -117,8 +117,10 @@ class Db {
 class DbEnv {
     friend class Db;
     friend class Dbc;
+    friend class DbTxn;
  public:
     DbEnv(u_int32_t flags);
+    ~DbEnv(void);
 
     DB_ENV *get_DB_ENV(void) {
 	if (this==0) return 0;
@@ -145,6 +147,7 @@ class DbEnv {
 
     DbEnv(DB_ENV *, u_int32_t /*flags*/);
     int maybe_throw_error(int /*err*/) throw (DbException);
+    static int maybe_throw_error(int, DbEnv*, int /*no_exceptions*/) throw (DbException);
 };
 
 	
@@ -161,6 +164,7 @@ class DbTxn {
     DbTxn(DB_TXN*);
  private:
     DB_TXN *the_txn;
+    
 };
 
 class Dbc : protected DBC