From 7119bbf096b6bdad2e81e54e55b7943de2f4ce86 Mon Sep 17 00:00:00 2001
From: Calin Culianu <calin.culianu@gmail.com>
Date: Thu, 15 Apr 2021 12:06:49 -0600
Subject: [PATCH] util: Refactor the Defer class to a header to be reusable

Summary
---

This commit adds a template class called `Defer` to the `util/` subdirectory.

This class is designed to leverage RAII and execute a lambda at scope
end.

It is used by passing it a lambda (or `std::function`) at construction time
which it will execute when the `Defer` instance is destructed.  This class can
be used as a language idiom to defer the execution of cleanup code at scope end
-- even if an exception is thrown.

Motivation: I noticed in various places in the codebase, very critical
cleanup code is manually executed at scope end -- however if an
exception is thrown by the code, the cleanup code will never run thus
leading to `assert` failures or potential subtle bugs and resource leaks.

What's worse, in some places the critical cleanup code is copy-pased in
several places.

Using this `Defer` idiom one can escape from all of this pain and also guarantee
safety.

This class was already used in this codebase in 2 places (added by me
redundantly in both places, private to each translation unit in which it
was added).

In this commit we just refactor it out to a publicly-visible header in
`util/defer.h` and thus we can re-use this idiom everywhere.

Should this MR be accepted and merged, I do plan on using this idiom in
the future in refactors and code cleanups or in new code.  (I already
know of 1 place where it should be added for safety.)

Test Plan
---

No semantic or other behavioral changes are introduced, however:

- `ninja all check-all`
---
 src/Makefile.am               |  1 +
 src/test/rpc_server_tests.cpp |  9 +--------
 src/util/defer.h              | 19 +++++++++++++++++++
 src/validation.cpp            | 11 +----------
 4 files changed, 22 insertions(+), 18 deletions(-)
 create mode 100644 src/util/defer.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 53698900dc..c0bd345ff7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -227,6 +227,7 @@ BITCOIN_CORE_H = \
   ui_interface.h \
   undo.h \
   util/bitmanip.h \
+  util/defer.h \
   util/moneystr.h \
   util/saltedhashers.h \
   util/system.h \
diff --git a/src/test/rpc_server_tests.cpp b/src/test/rpc_server_tests.cpp
index f806833a43..d23b060b22 100644
--- a/src/test/rpc_server_tests.cpp
+++ b/src/test/rpc_server_tests.cpp
@@ -8,13 +8,13 @@
 #include <chainparams.h>
 #include <config.h>
 #include <software_outdated.h>
+#include <util/defer.h>
 #include <util/system.h>
 
 #include <test/setup_common.h>
 
 #include <boost/test/unit_test.hpp>
 
-#include <functional>
 #include <string>
 
 BOOST_FIXTURE_TEST_SUITE(rpc_server_tests, TestingSetup)
@@ -38,13 +38,6 @@ static bool isRpcDisabled(const JSONRPCError &u) {
     return u.code == RPC_DISABLED;
 }
 
-struct Defer {
-    const std::function<void()> func;
-    template <typename F>
-    Defer(F && f) : func(f) {}
-    ~Defer() { func(); }
-};
-
 BOOST_AUTO_TEST_CASE(rpc_server_execute_command) {
     DummyConfig config;
     RPCServer rpcServer;
diff --git a/src/util/defer.h b/src/util/defer.h
new file mode 100644
index 0000000000..32ce524881
--- /dev/null
+++ b/src/util/defer.h
@@ -0,0 +1,19 @@
+// Copyright (c) 2021 The Bitcoin developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_DEFER_H
+#define BITCOIN_UTIL_DEFER_H
+
+#include <utility>
+
+/// Leverage RAII to run a functor at scope end
+template <typename Func>
+struct Defer {
+    Func func;
+    Defer(Func && f) : func(std::move(f)) {}
+    ~Defer() { func(); }
+};
+
+
+#endif // BITCOIN_UTIL_DEFER_H
diff --git a/src/validation.cpp b/src/validation.cpp
index 2b498c0f7c..8977cc15c9 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -44,6 +44,7 @@
 #include <txmempool.h>
 #include <ui_interface.h>
 #include <undo.h>
+#include <util/defer.h>
 #include <util/moneystr.h>
 #include <util/strencodings.h>
 #include <util/system.h>
@@ -3178,16 +3179,6 @@ bool PreciousBlock(const Config &config, CValidationState &state,
     return g_chainstate.PreciousBlock(config, state, pindex);
 }
 
-namespace {
-// Leverage RAII to run a functor at scope end
-template <typename Func>
-struct Defer {
-    Func func;
-    Defer(Func && f) : func(std::move(f)) {}
-    ~Defer() { func(); }
-};
-} // namespace
-
 bool CChainState::UnwindBlock(const Config &config, CValidationState &state,
                               CBlockIndex *pindex, bool invalidate) {
     CBlockIndex *to_mark_failed_or_parked = pindex;