From 5ce43da03dff3ee655949cd53d952dace555e268 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 15 Sep 2015 01:39:12 +0200 Subject: [PATCH 1/4] init: Ignore SIGPIPE Ignore SIGPIPE on all non-win32 OSes, otherwise an unexpectedly disconnecting RPC client will terminate the application. This problem was introduced with the libhttp-based RPC server. Fixes #6660. --- src/init.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index a12e38ff5..276effe58 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -687,10 +687,8 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) sa_hup.sa_flags = 0; sigaction(SIGHUP, &sa_hup, NULL); -#if defined (__SVR4) && defined (__sun) - // ignore SIGPIPE on Solaris + // Ignore SIGPIPE, otherwise it will bring the daemon down if the client closes unexpectedly signal(SIGPIPE, SIG_IGN); -#endif #endif // ********************************************************* Step 2: parameter interactions From 8b2d6edaa9fbfb6344ca51edd0b3655b451cbcac Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 18 Sep 2015 14:58:08 +0200 Subject: [PATCH 2/4] http: Disable libevent debug logging, if not explicitly enabled Add a option "-debug=libevent" to enable libevent debugging for troubleshooting. Libevent logging is redirected to our own log. --- src/httpserver.cpp | 19 +++++++++++++++++++ src/init.cpp | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index baca00757..3fc880214 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -320,6 +320,15 @@ static void HTTPWorkQueueRun(WorkQueue* queue) queue->Run(); } +/** libevent event log callback */ +static void libevent_log_cb(int severity, const char *msg) +{ + if (severity >= EVENT_LOG_WARN) // Log warn messages and higher without debug category + LogPrintf("libevent: %s\n", msg); + else + LogPrint("libevent", "libevent: %s\n", msg); +} + bool InitHTTPServer() { struct evhttp* http = 0; @@ -335,6 +344,16 @@ bool InitHTTPServer() return false; } + // Redirect libevent's logging to our own log + event_set_log_callback(&libevent_log_cb); +#if LIBEVENT_VERSION_NUMBER >= 0x02010100 + // If -debug=libevent, set full libevent debugging. + // Otherwise, disable all libevent debugging. + if (LogAcceptCategory("libevent")) + event_enable_debug_logging(EVENT_DBG_ALL); + else + event_enable_debug_logging(EVENT_DBG_NONE); +#endif #ifdef WIN32 evthread_use_windows_threads(); #else diff --git a/src/init.cpp b/src/init.cpp index 276effe58..c259e3fa2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -387,7 +387,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-flushwallet", strprintf("Run a thread to flush wallet periodically (default: %u)", 1)); strUsage += HelpMessageOpt("-stopafterblockimport", strprintf("Stop running after importing blocks from disk (default: %u)", 0)); } - string debugCategories = "addrman, alert, bench, coindb, db, lock, rand, rpc, selectcoins, mempool, mempoolrej, net, proxy, prune, http"; // Don't translate these and qt below + string debugCategories = "addrman, alert, bench, coindb, db, lock, rand, rpc, selectcoins, mempool, mempoolrej, net, proxy, prune, http, libevent"; // Don't translate these and qt below if (mode == HMM_BITCOIN_QT) debugCategories += ", qt"; strUsage += HelpMessageOpt("-debug=", strprintf(_("Output debugging information (default: %u, supplying is optional)"), 0) + ". " + From 2190ea6c4e7f56f29fc18539284801a5f504ee48 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 18 Sep 2015 15:45:38 +0200 Subject: [PATCH 3/4] rpc: Split option -rpctimeout into -rpcservertimeout and -rpcclienttimeout The two timeouts for the server and client, are essentially different: - In the case of the server it should be a lower value to avoid clients clogging up connection slots - In the case of the client it should be a high value to accomedate slow responses from the server, for example for slow queries or when the lock is contended Split the options into `-rpcservertimeout` and `-rpcclienttimeout` with respective defaults of 30 and 900. --- contrib/debian/examples/bitcoin.conf | 2 +- src/bitcoin-cli.cpp | 5 ++++- src/httpserver.cpp | 2 +- src/httpserver.h | 2 +- src/init.cpp | 2 +- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/contrib/debian/examples/bitcoin.conf b/contrib/debian/examples/bitcoin.conf index 62ffd7123..6aae4ad4d 100644 --- a/contrib/debian/examples/bitcoin.conf +++ b/contrib/debian/examples/bitcoin.conf @@ -73,7 +73,7 @@ # How many seconds bitcoin will wait for a complete RPC HTTP request. # after the HTTP connection is established. -#rpctimeout=30 +#rpcclienttimeout=30 # By default, only RPC connections from localhost are allowed. # Specify as many rpcallowip= settings as you like to allow connections from other hosts, diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 866c6f2d4..7839b3b6b 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -22,6 +22,8 @@ using namespace std; +static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900; + std::string HelpMessageCli() { string strUsage; @@ -37,6 +39,7 @@ std::string HelpMessageCli() strUsage += HelpMessageOpt("-rpcwait", _("Wait for RPC server to start")); strUsage += HelpMessageOpt("-rpcuser=", _("Username for JSON-RPC connections")); strUsage += HelpMessageOpt("-rpcpassword=", _("Password for JSON-RPC connections")); + strUsage += HelpMessageOpt("-rpcclienttimeout=", strprintf(_("Timeout during HTTP requests (default: %d)"), DEFAULT_HTTP_CLIENT_TIMEOUT)); return strUsage; } @@ -150,7 +153,7 @@ UniValue CallRPC(const string& strMethod, const UniValue& params) struct evhttp_connection *evcon = evhttp_connection_base_new(base, NULL, host.c_str(), port); // TODO RAII if (evcon == NULL) throw runtime_error("create connection failed"); - evhttp_connection_set_timeout(evcon, GetArg("-rpctimeout", 30)); + evhttp_connection_set_timeout(evcon, GetArg("-rpcclienttimeout", DEFAULT_HTTP_CLIENT_TIMEOUT)); HTTPReply response; struct evhttp_request *req = evhttp_request_new(http_request_done, (void*)&response); // TODO RAII diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 3fc880214..600e57b7c 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -374,7 +374,7 @@ bool InitHTTPServer() return false; } - evhttp_set_timeout(http, GetArg("-rpctimeout", DEFAULT_HTTP_TIMEOUT)); + evhttp_set_timeout(http, GetArg("-rpcservertimeout", DEFAULT_HTTP_SERVER_TIMEOUT)); evhttp_set_max_body_size(http, MAX_SIZE); evhttp_set_gencb(http, http_request_cb, NULL); diff --git a/src/httpserver.h b/src/httpserver.h index 459c60c04..b377dc19f 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -13,7 +13,7 @@ static const int DEFAULT_HTTP_THREADS=4; static const int DEFAULT_HTTP_WORKQUEUE=16; -static const int DEFAULT_HTTP_TIMEOUT=30; +static const int DEFAULT_HTTP_SERVER_TIMEOUT=30; struct evhttp_request; struct event_base; diff --git a/src/init.cpp b/src/init.cpp index c259e3fa2..3b35fb380 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -440,7 +440,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-rpcthreads=", strprintf(_("Set the number of threads to service RPC calls (default: %d)"), DEFAULT_HTTP_THREADS)); if (showDebug) { strUsage += HelpMessageOpt("-rpcworkqueue=", strprintf("Set the depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE)); - strUsage += HelpMessageOpt("-rpctimeout=", strprintf("Timeout during HTTP requests (default: %d)", DEFAULT_HTTP_TIMEOUT)); + strUsage += HelpMessageOpt("-rpcservertimeout=", strprintf("Timeout during HTTP requests (default: %d)", DEFAULT_HTTP_SERVER_TIMEOUT)); } if (mode == HMM_BITCOIN_QT) From ddf98d1d84343f2db0502a85628ef80f2ec57dbd Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sat, 19 Sep 2015 14:12:44 +0200 Subject: [PATCH 4/4] Make RPC tests cope with server-side timeout between requests Python's httplib does not graciously handle disconnections from the http server, resulting in BadStatusLine errors. See https://bugs.python.org/issue3566 "httplib persistent connections violate MUST in RFC2616 sec 8.1.4." This was fixed in Python 3.5. Work around it for now. --- qa/rpc-tests/test_framework/authproxy.py | 36 +++++++++++++++--------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/qa/rpc-tests/test_framework/authproxy.py b/qa/rpc-tests/test_framework/authproxy.py index bc7d655fd..33014dc13 100644 --- a/qa/rpc-tests/test_framework/authproxy.py +++ b/qa/rpc-tests/test_framework/authproxy.py @@ -106,6 +106,26 @@ class AuthServiceProxy(object): name = "%s.%s" % (self.__service_name, name) return AuthServiceProxy(self.__service_url, name, connection=self.__conn) + def _request(self, method, path, postdata): + ''' + Do a HTTP request, with retry if we get disconnected (e.g. due to a timeout). + This is a workaround for https://bugs.python.org/issue3566 which is fixed in Python 3.5. + ''' + headers = {'Host': self.__url.hostname, + 'User-Agent': USER_AGENT, + 'Authorization': self.__auth_header, + 'Content-type': 'application/json'} + try: + self.__conn.request(method, path, postdata, headers) + return self._get_response() + except httplib.BadStatusLine as e: + if e.line == "''": # if connection was closed, try again + self.__conn.close() + self.__conn.request(method, path, postdata, headers) + return self._get_response() + else: + raise + def __call__(self, *args): AuthServiceProxy.__id_count += 1 @@ -115,13 +135,7 @@ class AuthServiceProxy(object): 'method': self.__service_name, 'params': args, 'id': AuthServiceProxy.__id_count}, default=EncodeDecimal) - self.__conn.request('POST', self.__url.path, postdata, - {'Host': self.__url.hostname, - 'User-Agent': USER_AGENT, - 'Authorization': self.__auth_header, - 'Content-type': 'application/json'}) - - response = self._get_response() + response = self._request('POST', self.__url.path, postdata) if response['error'] is not None: raise JSONRPCException(response['error']) elif 'result' not in response: @@ -133,13 +147,7 @@ class AuthServiceProxy(object): def _batch(self, rpc_call_list): postdata = json.dumps(list(rpc_call_list), default=EncodeDecimal) log.debug("--> "+postdata) - self.__conn.request('POST', self.__url.path, postdata, - {'Host': self.__url.hostname, - 'User-Agent': USER_AGENT, - 'Authorization': self.__auth_header, - 'Content-type': 'application/json'}) - - return self._get_response() + return self._request('POST', self.__url.path, postdata) def _get_response(self): http_response = self.__conn.getresponse()