pivx-cli better error handling + libevent RAII upstream backports

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

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 \

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 \

src/httpserver.cpp

@@ -22,14 +22,14 @@
 #include <signal.h>
 #include <future>
 
-#include <event2/event.h>
-#include <event2/http.h>
 #include <event2/thread.h>
 #include <event2/buffer.h>
 #include <event2/bufferevent.h>
 #include <event2/util.h>
 #include <event2/keyvalq_struct.h>
 
+#include "support/events.h"
+
 #ifdef EVENT__HAVE_NETINET_IN_H
 #include <netinet/in.h>
 #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<HTTPClosure>(workQueueDepth);
-    eventBase = base;
-    eventHTTP = http;
+    // tranfer ownership to eventBase/HTTP via .release()
+    eventBase = base_ctr.release();
+    eventHTTP = http_ctr.release();
     return true;
 }
 

src/pivx-cli.cpp

@@ -20,10 +20,9 @@
 #include <stdio.h>
 #include <tuple>
 
-#include <event2/event.h>
-#include <event2/http.h>
 #include <event2/buffer.h>
 #include <event2/keyvalq_struct.h>
+#include "support/events.h"
 
 #include <univalue.h>
 
@@ -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<HTTPReply*>(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<HTTPReply*>(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,15 +220,15 @@ 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");
     evhttp_add_header(output_headers, "Authorization", (std::string("Basic ") + EncodeBase64(strRPCUserColonPass)).c_str());
 
     // 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))

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 <ios>
+#include <memory>
+
+#include <event2/event.h>
+#include <event2/http.h>
+
+#define MAKE_RAII(type) \
+/* deleter */\
+struct type##_deleter {\
+    void operator()(struct type* ob) {\
+        type##_free(ob);\
+    }\
+};\
+/* unique ptr typedef */\
+typedef std::unique_ptr<struct type, type##_deleter> 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

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