diff --git a/CMakeLists.txt b/CMakeLists.txt index 78917f71531ba..82d8307fa545e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -180,6 +180,7 @@ source_group("BitcoinHeaders" FILES ${IMMER_HEADERS} ${EVO_HEADERS} ./src/support/cleanse.h + ./src/support/events.h ) set(SERVER_SOURCES diff --git a/src/Makefile.am b/src/Makefile.am index 354a66027a035..05965af15be57 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -273,6 +273,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/Makefile.test.include b/src/Makefile.test.include index e904d5a1ea4d8..b90631b6062bd 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -128,6 +128,7 @@ BITCOIN_TESTS =\ test/pmt_tests.cpp \ test/policyestimator_tests.cpp \ test/prevector_tests.cpp \ + test/raii_event_tests.cpp \ test/random_tests.cpp \ test/reverselock_tests.cpp \ test/rpc_tests.cpp \ diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 65418238a6844..54ff92d4bdfb3 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -22,14 +22,14 @@ #include #include -#include -#include #include #include #include #include #include +#include "support/events.h" + #ifdef EVENT__HAVE_NETINET_IN_H #include #ifdef _XOPEN_SOURCE_EXTENDED @@ -354,9 +354,6 @@ static void libevent_log_cb(int severity, const char *msg) bool InitHTTPServer() { - struct evhttp* http = 0; - struct event_base* base = 0; - if (!InitHTTPAllowList()) return false; @@ -382,17 +379,13 @@ bool InitHTTPServer() evthread_use_pthreads(); #endif - base = event_base_new(); // XXX RAII - if (!base) { - LogPrintf("Couldn't create an event_base: exiting\n"); - return false; - } + raii_event_base base_ctr = obtain_event_base(); /* Create a new evhttp object to handle requests. */ - http = evhttp_new(base); // XXX RAII + raii_evhttp http_ctr = obtain_evhttp(base_ctr.get()); + struct evhttp* http = http_ctr.get(); if (!http) { LogPrintf("couldn't create evhttp. Exiting.\n"); - event_base_free(base); return false; } @@ -403,8 +396,6 @@ bool InitHTTPServer() if (!HTTPBindAddresses(http)) { LogPrintf("Unable to bind any endpoint for RPC server\n"); - evhttp_free(http); - event_base_free(base); return false; } @@ -413,8 +404,9 @@ bool InitHTTPServer() LogPrintf("HTTP: creating work queue of depth %d\n", workQueueDepth); workQueue = new WorkQueue(workQueueDepth); - eventBase = base; - eventHTTP = http; + // tranfer ownership to eventBase/HTTP via .release() + eventBase = base_ctr.release(); + eventHTTP = http_ctr.release(); return true; } diff --git a/src/pivx-cli.cpp b/src/pivx-cli.cpp index d815df8d73a5f..80d68c03a3836 100644 --- a/src/pivx-cli.cpp +++ b/src/pivx-cli.cpp @@ -20,10 +20,9 @@ #include #include -#include -#include #include #include +#include "support/events.h" #include @@ -53,6 +52,18 @@ std::string HelpMessageCli() return strUsage; } +/** libevent event log callback */ +static void libevent_log_cb(int severity, const char *msg) +{ +#ifndef EVENT_LOG_ERR // EVENT_LOG_ERR was added in 2.0.19; but before then _EVENT_LOG_ERR existed. +# define EVENT_LOG_ERR _EVENT_LOG_ERR +#endif + // Ignore everything other than errors + if (severity >= EVENT_LOG_ERR) { + throw std::runtime_error(strprintf("libevent error: %s", msg)); + } +} + ////////////////////////////////////////////////////////////////////////////// // // Start @@ -119,17 +130,40 @@ static bool AppInitRPC(int argc, char* argv[]) /** Reply structure for request_done to fill in */ struct HTTPReply { + HTTPReply(): status(0), error(-1) {} + int status; + int error; std::string body; }; +const char *http_errorstring(int code) +{ + switch(code) { + case EVREQ_HTTP_TIMEOUT: + return "timeout reached"; + case EVREQ_HTTP_EOF: + return "EOF reached"; + case EVREQ_HTTP_INVALID_HEADER: + return "error while reading header, or invalid header"; + case EVREQ_HTTP_BUFFER_ERROR: + return "error encountered while reading or writing"; + case EVREQ_HTTP_REQUEST_CANCEL: + return "request was canceled"; + case EVREQ_HTTP_DATA_TOO_LONG: + return "response body is larger than allowed"; + default: + return "unknown"; + } +} + static void http_request_done(struct evhttp_request *req, void *ctx) { HTTPReply *reply = static_cast(ctx); if (req == NULL) { - /* If req is NULL, it means an error occurred while connecting, but - * I'm not sure how to find out which one. We also don't really care. + /* If req is NULL, it means an error occurred while connecting: the + * error code will have been passed to http_error_cb. */ reply->status = 0; return; @@ -148,26 +182,29 @@ static void http_request_done(struct evhttp_request *req, void *ctx) } } +static void http_error_cb(enum evhttp_request_error err, void *ctx) +{ + HTTPReply *reply = static_cast(ctx); + reply->error = err; +} + UniValue CallRPC(const std::string& strMethod, const UniValue& params) { std::string host = gArgs.GetArg("-rpcconnect", DEFAULT_RPCCONNECT); int port = gArgs.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, gArgs.GetArg("-rpcclienttimeout", DEFAULT_HTTP_CLIENT_TIMEOUT)); + raii_evhttp_connection evcon = obtain_evhttp_connection_base(base.get(), host, port); + evhttp_connection_set_timeout(evcon.get(), gArgs.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"); + evhttp_request_set_error_cb(req.get(), http_error_cb); // Get credentials std::string strRPCUserColonPass; @@ -183,7 +220,7 @@ UniValue CallRPC(const std::string& strMethod, const UniValue& params) strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-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"); @@ -191,7 +228,7 @@ 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()); @@ -207,19 +244,16 @@ UniValue CallRPC(const std::string& strMethod, const UniValue& params) throw CConnectionFailed("uri-encode failed"); } } - int r = evhttp_make_request(evcon, req, EVHTTP_REQ_POST, endpoint.c_str()); + 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("couldn't connect to server"); + 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)); else if (response.status == HTTP_UNAUTHORIZED) throw std::runtime_error("incorrect rpcuser or rpcpassword (authorization failed)"); else if (response.status >= 400 && response.status != HTTP_BAD_REQUEST && response.status != HTTP_NOT_FOUND && response.status != HTTP_INTERNAL_SERVER_ERROR) @@ -335,8 +369,9 @@ int main(int argc, char* argv[]) SetupEnvironment(); if (!SetupNetworking()) { fprintf(stderr, "Error: Initializing networking failed\n"); - exit(1); + return EXIT_FAILURE; } + event_set_log_callback(&libevent_log_cb); try { if (!AppInitRPC(argc, argv)) diff --git a/src/support/events.h b/src/support/events.h new file mode 100644 index 0000000000000..90690876eee05 --- /dev/null +++ b/src/support/events.h @@ -0,0 +1,56 @@ +// 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 +#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); + +inline 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; +} + +inline 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)); +} + +inline raii_evhttp obtain_evhttp(struct event_base* base) { + return raii_evhttp(evhttp_new(base)); +} + +inline raii_evhttp_request obtain_evhttp_request(void(*cb)(struct evhttp_request *, void *), void *arg) { + return raii_evhttp_request(evhttp_request_new(cb, arg)); +} + +inline 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 diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 6345f4981ec27..5d4b726fea4dc 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -145,6 +145,7 @@ set(BITCOIN_TESTS ${CMAKE_CURRENT_SOURCE_DIR}/pmt_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/policyestimator_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/prevector_tests.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/raii_event_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/random_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/reverselock_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/rpc_tests.cpp diff --git a/src/test/raii_event_tests.cpp b/src/test/raii_event_tests.cpp new file mode 100644 index 0000000000000..036c42e24d447 --- /dev/null +++ b/src/test/raii_event_tests.cpp @@ -0,0 +1,92 @@ +// 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 + +#ifdef EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED +// It would probably be ideal to define dummy test(s) that report skipped, but boost::test doesn't seem to make that practical (at least not in versions available with common distros) + +#include +#include + +#include "support/events.h" + +#include "test/test_pivx.h" + +#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 = nullptr; + { + 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 = nullptr; + { + auto base = obtain_event_base(); + auto event = obtain_event(base.get(), -1, 0, nullptr, nullptr); + + 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 = nullptr; + void* event_ptr = nullptr; + { + auto base = obtain_event_base(); + auto event = obtain_event(base.get(), -1, 0, nullptr, nullptr); + + 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() + +#endif // EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED