Making use of unique_ptr to ensure environment handle never leaks.

nanocurrency · Oct 27, 2024 · 5ca6ff7 · 5ca6ff7
1 parent 1fce19b
commit 5ca6ff7
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 11 deletions.
diff --git a/nano/store/lmdb/lmdb.cpp b/nano/store/lmdb/lmdb.cpp
@@ -121,9 +121,7 @@ bool nano::store::lmdb::component::vacuum_after_upgrade (std::filesystem::path c
 	if (vacuum_success)
 	{
 		// Need to close the database to release the file handle
-		mdb_env_sync (env.environment, true);
-		mdb_env_close (env.environment);
-		env.environment = nullptr;
+		mdb_env_sync (env, true);
 
 		// Replace the ledger file with the vacuumed one
 		std::filesystem::rename (vacuum_path, path_a);
@@ -155,7 +153,7 @@ void nano::store::lmdb::component::serialize_mdb_tracker (boost::property_tree::
 void nano::store::lmdb::component::serialize_memory_stats (boost::property_tree::ptree & json)
 {
 	MDB_stat stats;
-	auto status (mdb_env_stat (env.environment, &stats));
+	auto status (mdb_env_stat (env, &stats));
 	release_assert (status == 0);
 	json.put ("branch_pages", stats.ms_branch_pages);
 	json.put ("depth", stats.ms_depth);
@@ -448,7 +446,7 @@ std::string nano::store::lmdb::component::error_string (int status) const
 
 bool nano::store::lmdb::component::copy_db (std::filesystem::path const & destination_file)
 {
-	return !mdb_env_copy2 (env.environment, destination_file.string ().c_str (), MDB_CP_COMPACT);
+	return !mdb_env_copy2 (env, destination_file.string ().c_str (), MDB_CP_COMPACT);
 }
 
 void nano::store::lmdb::component::rebuild_db (store::write_transaction const & transaction_a)

diff --git a/nano/store/lmdb/lmdb_env.cpp b/nano/store/lmdb/lmdb_env.cpp
@@ -19,8 +19,10 @@ void nano::store::lmdb::env::init (bool & error_a, std::filesystem::path const &
 		nano::set_secure_perm_directory (path_a.parent_path (), error_chmod);
 		if (!error_mkdir)
 		{
+			MDB_env * environment;
 			auto status1 (mdb_env_create (&environment));
 			release_assert (status1 == 0);
+			this->environment.reset (environment);
 			auto status2 (mdb_env_set_maxdbs (environment, options_a.config.max_databases));
 			release_assert (status2 == 0);
 			auto map_size = options_a.config.map_size;
@@ -66,13 +68,11 @@ void nano::store::lmdb::env::init (bool & error_a, std::filesystem::path const &
 		else
 		{
 			error_a = true;
-			environment = nullptr;
 		}
 	}
 	else
 	{
 		error_a = true;
-		environment = nullptr;
 	}
 }
 
@@ -81,14 +81,13 @@ nano::store::lmdb::env::~env ()
 	if (environment != nullptr)
 	{
 		// Make sure the commits are flushed. This is a no-op unless MDB_NOSYNC is used.
-		mdb_env_sync (environment, true);
-		mdb_env_close (environment);
+		mdb_env_sync (environment.get (), true);
 	}
 }
 
 nano::store::lmdb::env::operator MDB_env * () const
 {
-	return environment;
+	return environment.get ();
 }
 
 nano::store::read_transaction nano::store::lmdb::env::tx_begin_read (store::lmdb::txn_callbacks mdb_txn_callbacks) const

diff --git a/nano/store/lmdb/lmdb_env.hpp b/nano/store/lmdb/lmdb_env.hpp
@@ -62,7 +62,7 @@ class env final
 	store::read_transaction tx_begin_read (txn_callbacks callbacks = txn_callbacks{}) const;
 	store::write_transaction tx_begin_write (txn_callbacks callbacks = txn_callbacks{}) const;
 	MDB_txn * tx (store::transaction const & transaction_a) const;
-	MDB_env * environment;
+	std::unique_ptr<MDB_env, decltype (&mdb_env_close)> environment{ nullptr, mdb_env_close };
 	nano::id_t const store_id{ nano::next_id () };
 };
 } // namespace nano::store::lmdb