From e5534d2f0198e341c40e4ffa606fb65f3cd06863 Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Tue, 20 Dec 2016 15:59:42 +0900 Subject: [PATCH 1/4] Added std::unique_ptr<> wrappers with deleters for libevent modules. --- src/Makefile.am | 1 + src/support/events.h | 55 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 src/support/events.h diff --git a/src/Makefile.am b/src/Makefile.am index c7124c3ef..ac75f250a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -134,6 +134,7 @@ BITCOIN_CORE_H = \ support/allocators/secure.h \ support/allocators/zeroafterfree.h \ support/cleanse.h \ + support/events.h \ support/lockedpool.h \ sync.h \ threadsafety.h \ diff --git a/src/support/events.h b/src/support/events.h new file mode 100644 index 000000000..16378086f --- /dev/null +++ b/src/support/events.h @@ -0,0 +1,55 @@ +// Copyright (c) 2016 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_SUPPORT_EVENTS_H +#define BITCOIN_SUPPORT_EVENTS_H + +#include + +#include +#include + +#define MAKE_RAII(type) \ +/* deleter */\ +struct type##_deleter {\ + void operator()(struct type* ob) {\ + type##_free(ob);\ + }\ +};\ +/* unique ptr typedef */\ +typedef std::unique_ptr raii_##type + +MAKE_RAII(event_base); +MAKE_RAII(event); +MAKE_RAII(evhttp); +MAKE_RAII(evhttp_request); +MAKE_RAII(evhttp_connection); + +raii_event_base obtain_event_base() { + auto result = raii_event_base(event_base_new()); + if (!result.get()) + throw std::runtime_error("cannot create event_base"); + return result; +} + +raii_event obtain_event(struct event_base* base, evutil_socket_t s, short events, event_callback_fn cb, void* arg) { + return raii_event(event_new(base, s, events, cb, arg)); +} + +raii_evhttp obtain_evhttp(struct event_base* base) { + return raii_evhttp(evhttp_new(base)); +} + +raii_evhttp_request obtain_evhttp_request(void(*cb)(struct evhttp_request *, void *), void *arg) { + return raii_evhttp_request(evhttp_request_new(cb, arg)); +} + +raii_evhttp_connection obtain_evhttp_connection_base(struct event_base* base, std::string host, uint16_t port) { + auto result = raii_evhttp_connection(evhttp_connection_base_new(base, NULL, host.c_str(), port)); + if (!result.get()) + throw std::runtime_error("create connection failed"); + return result; +} + +#endif // BITCOIN_SUPPORT_EVENTS_H From 7f7f102b8d5024cc702e308f2ad86a4137d790f7 Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Tue, 20 Dec 2016 20:46:11 +0900 Subject: [PATCH 2/4] Switched bitcoin-cli.cpp to use RAII unique pointers with deleters. --- src/bitcoin-cli.cpp | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 596cc8ead..2d11f4530 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -17,10 +17,9 @@ #include #include -#include -#include #include #include +#include "support/events.h" #include @@ -190,23 +189,19 @@ UniValue CallRPC(const std::string& strMethod, const UniValue& params) std::string host = GetArg("-rpcconnect", DEFAULT_RPCCONNECT); int port = GetArg("-rpcport", BaseParams().RPCPort()); - // Create event base - struct event_base *base = event_base_new(); // TODO RAII - if (!base) - throw std::runtime_error("cannot create event_base"); + // Obtain event base + raii_event_base base = obtain_event_base(); // Synchronously look up hostname - struct evhttp_connection *evcon = evhttp_connection_base_new(base, NULL, host.c_str(), port); // TODO RAII - if (evcon == NULL) - throw std::runtime_error("create connection failed"); - evhttp_connection_set_timeout(evcon, GetArg("-rpcclienttimeout", DEFAULT_HTTP_CLIENT_TIMEOUT)); + raii_evhttp_connection evcon = obtain_evhttp_connection_base(base.get(), host, port); + evhttp_connection_set_timeout(evcon.get(), GetArg("-rpcclienttimeout", DEFAULT_HTTP_CLIENT_TIMEOUT)); HTTPReply response; - struct evhttp_request *req = evhttp_request_new(http_request_done, (void*)&response); // TODO RAII + raii_evhttp_request req = obtain_evhttp_request(http_request_done, (void*)&response); if (req == NULL) throw std::runtime_error("create http request failed"); #if LIBEVENT_VERSION_NUMBER >= 0x02010300 - evhttp_request_set_error_cb(req, http_error_cb); + evhttp_request_set_error_cb(req.get(), http_error_cb); #endif // Get credentials @@ -223,7 +218,7 @@ UniValue CallRPC(const std::string& strMethod, const UniValue& params) strRPCUserColonPass = mapArgs["-rpcuser"] + ":" + mapArgs["-rpcpassword"]; } - struct evkeyvalq *output_headers = evhttp_request_get_output_headers(req); + struct evkeyvalq* output_headers = evhttp_request_get_output_headers(req.get()); assert(output_headers); evhttp_add_header(output_headers, "Host", host.c_str()); evhttp_add_header(output_headers, "Connection", "close"); @@ -231,20 +226,17 @@ UniValue CallRPC(const std::string& strMethod, const UniValue& params) // Attach request data std::string strRequest = JSONRPCRequestObj(strMethod, params, 1).write() + "\n"; - struct evbuffer * output_buffer = evhttp_request_get_output_buffer(req); + struct evbuffer* output_buffer = evhttp_request_get_output_buffer(req.get()); assert(output_buffer); evbuffer_add(output_buffer, strRequest.data(), strRequest.size()); - int r = evhttp_make_request(evcon, req, EVHTTP_REQ_POST, "/"); + int r = evhttp_make_request(evcon.get(), req.get(), EVHTTP_REQ_POST, "/"); + req.release(); // ownership moved to evcon in above call if (r != 0) { - evhttp_connection_free(evcon); - event_base_free(base); throw CConnectionFailed("send http request failed"); } - event_base_dispatch(base); - evhttp_connection_free(evcon); - event_base_free(base); + event_base_dispatch(base.get()); if (response.status == 0) throw CConnectionFailed(strprintf("couldn't connect to server: %s (code %d)\n(make sure server is running and you are connecting to the correct RPC port)", http_errorstring(response.error), response.error)); From 280a5599ebccbb54fb650f5e3939b0878b2f667a Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Wed, 21 Dec 2016 13:43:49 +0900 Subject: [PATCH 3/4] Added some simple tests for the RAII-style events. --- src/Makefile.test.include | 3 +- src/support/events.h | 1 + src/test/raii_event_tests.cpp | 88 +++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 src/test/raii_event_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 5969de087..5d57daff4 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -74,6 +74,7 @@ BITCOIN_TESTS =\ test/policyestimator_tests.cpp \ test/pow_tests.cpp \ test/prevector_tests.cpp \ + test/raii_event_tests.cpp \ test/reverselock_tests.cpp \ test/rpc_tests.cpp \ test/sanity_tests.cpp \ @@ -111,7 +112,7 @@ endif test_test_bitcoin_SOURCES = $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES) test_test_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -I$(builddir)/test/ $(TESTDEFS) test_test_bitcoin_LDADD = $(LIBBITCOIN_SERVER) $(LIBBITCOIN_CLI) $(LIBBITCOIN_COMMON) $(LIBBITCOIN_UTIL) $(LIBBITCOIN_CONSENSUS) $(LIBBITCOIN_CRYPTO) $(LIBUNIVALUE) $(LIBLEVELDB) $(LIBMEMENV) \ - $(BOOST_LIBS) $(BOOST_UNIT_TEST_FRAMEWORK_LIB) $(LIBSECP256K1) + $(BOOST_LIBS) $(BOOST_UNIT_TEST_FRAMEWORK_LIB) $(LIBSECP256K1) $(EVENT_LIBS) test_test_bitcoin_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) if ENABLE_WALLET test_test_bitcoin_LDADD += $(LIBBITCOIN_WALLET) diff --git a/src/support/events.h b/src/support/events.h index 16378086f..4f2f3cf9e 100644 --- a/src/support/events.h +++ b/src/support/events.h @@ -6,6 +6,7 @@ #define BITCOIN_SUPPORT_EVENTS_H #include +#include #include #include diff --git a/src/test/raii_event_tests.cpp b/src/test/raii_event_tests.cpp new file mode 100644 index 000000000..87d25c0e2 --- /dev/null +++ b/src/test/raii_event_tests.cpp @@ -0,0 +1,88 @@ +// Copyright (c) 2016 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include + +#include "support/events.h" + +#include "test/test_bitcoin.h" + +#include + +#include + +static std::map tags; +static std::map orders; +static uint16_t tagSequence = 0; + +static void* tag_malloc(size_t sz) { + void* mem = malloc(sz); + if (!mem) return mem; + tags[mem]++; + orders[mem] = tagSequence++; + return mem; +} + +static void tag_free(void* mem) { + tags[mem]--; + orders[mem] = tagSequence++; + free(mem); +} + +BOOST_FIXTURE_TEST_SUITE(raii_event_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(raii_event_creation) +{ + event_set_mem_functions(tag_malloc, realloc, tag_free); + + void* base_ptr = NULL; + { + auto base = obtain_event_base(); + base_ptr = (void*)base.get(); + BOOST_CHECK(tags[base_ptr] == 1); + } + BOOST_CHECK(tags[base_ptr] == 0); + + void* event_ptr = NULL; + { + auto base = obtain_event_base(); + auto event = obtain_event(base.get(), -1, 0, NULL, NULL); + + base_ptr = (void*)base.get(); + event_ptr = (void*)event.get(); + + BOOST_CHECK(tags[base_ptr] == 1); + BOOST_CHECK(tags[event_ptr] == 1); + } + BOOST_CHECK(tags[base_ptr] == 0); + BOOST_CHECK(tags[event_ptr] == 0); + + event_set_mem_functions(malloc, realloc, free); +} + +BOOST_AUTO_TEST_CASE(raii_event_order) +{ + event_set_mem_functions(tag_malloc, realloc, tag_free); + + void* base_ptr = NULL; + void* event_ptr = NULL; + { + auto base = obtain_event_base(); + auto event = obtain_event(base.get(), -1, 0, NULL, NULL); + + base_ptr = (void*)base.get(); + event_ptr = (void*)event.get(); + + // base should have allocated before event + BOOST_CHECK(orders[base_ptr] < orders[event_ptr]); + } + // base should be freed after event + BOOST_CHECK(orders[base_ptr] > orders[event_ptr]); + + event_set_mem_functions(malloc, realloc, free); +} + +BOOST_AUTO_TEST_SUITE_END() From 05a55a639b6cd01b06be285db53edccdaf2ca189 Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Wed, 4 Jan 2017 18:16:47 +0900 Subject: [PATCH 4/4] Added EVENT_CFLAGS to test makefile to explicitly include libevent headers. --- src/Makefile.test.include | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 5d57daff4..c55c3ab46 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -110,7 +110,7 @@ BITCOIN_TESTS += \ endif test_test_bitcoin_SOURCES = $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES) -test_test_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -I$(builddir)/test/ $(TESTDEFS) +test_test_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -I$(builddir)/test/ $(TESTDEFS) $(EVENT_CFLAGS) test_test_bitcoin_LDADD = $(LIBBITCOIN_SERVER) $(LIBBITCOIN_CLI) $(LIBBITCOIN_COMMON) $(LIBBITCOIN_UTIL) $(LIBBITCOIN_CONSENSUS) $(LIBBITCOIN_CRYPTO) $(LIBUNIVALUE) $(LIBLEVELDB) $(LIBMEMENV) \ $(BOOST_LIBS) $(BOOST_UNIT_TEST_FRAMEWORK_LIB) $(LIBSECP256K1) $(EVENT_LIBS) test_test_bitcoin_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)