From 4832d777e389369d704652aa585f5f3bc2265187 Mon Sep 17 00:00:00 2001 From: Marco Nilsson Date: Sat, 13 Jan 2018 12:46:56 +0100 Subject: [PATCH 1/9] Move block poll back to after AppInit2 and return false on daemon forks. AppInit2 is also used by the Qt initializer so blocking in the AppInit2 function blocks the UI from starting up. This solution isn't ideal either so we should return an enum from the init to let the caller know which actions to take. --- src/init.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 14e2ddeb15..d6b2574585 100755 --- a/src/init.cpp +++ b/src/init.cpp @@ -182,7 +182,6 @@ bool AppInit(int argc, char* argv[]) exit(ret); } - // Blocking call until shutdown is requested fRet = AppInit2(threads); } catch (std::exception& e) { @@ -195,6 +194,13 @@ bool AppInit(int argc, char* argv[]) PrintException(NULL, "AppInit()"); } + // Succesfully initialized, wait for shutdown + if(fRet) + { + while (!ShutdownRequested()) + MilliSleep(500); + } + Shutdown(NULL); // delete thread handler @@ -601,7 +607,12 @@ bool AppInit2(ThreadHandlerPtr threads) if (pid > 0) { CreatePidFile(GetPidFile(), pid); - return true; + + // While this is technically successful we need to return false + // in order to shut down the parent process. This can be improved + // by either returning an enum or checking if the current process + // is a child process. + return false; } pid_t sid = setsid(); @@ -1025,9 +1036,5 @@ bool AppInit2(ThreadHandlerPtr threads) // Add wallet transactions that aren't already in a block to mapTransactions pwalletMain->ReacceptWalletTransactions(); - // Succesfully initialized, wait for shutdown - while (!ShutdownRequested()) - MilliSleep(500); - return true; } From e790c15aed19172ed4fdba61fbdcb05b9fcf3876 Mon Sep 17 00:00:00 2001 From: Marco Nilsson Date: Sat, 13 Jan 2018 12:48:48 +0100 Subject: [PATCH 2/9] Update changelog and version. --- CHANGELOG.md | 4 ++++ src/clientversion.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c06a2af103..4bd2a4f45c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [3.7.3.0] 2018-01-13 +### Fixed + - Fix for UI getting stuck in splash screen (@denravonska). + ## [3.7.2.0] 2018-01-13 ### Fixed - Properly fix for wallet not daemonizing, #822 (@denravonska). diff --git a/src/clientversion.h b/src/clientversion.h index 10a88f7457..81e8dc00d8 100644 --- a/src/clientversion.h +++ b/src/clientversion.h @@ -8,7 +8,7 @@ // These need to be macros, as version.cpp's and bitcoin-qt.rc's voodoo requires it #define CLIENT_VERSION_MAJOR 3 #define CLIENT_VERSION_MINOR 7 -#define CLIENT_VERSION_REVISION 2 +#define CLIENT_VERSION_REVISION 3 #define CLIENT_VERSION_BUILD 0 // Converts the parameter X to a string after macro replacement on X has been performed. From af9d1a27be70b97d181e64306d79a56c615f01a8 Mon Sep 17 00:00:00 2001 From: Marco Nilsson Date: Sun, 14 Jan 2018 21:45:16 +0100 Subject: [PATCH 3/9] Request shutdown after forking instead of returning an error. --- src/init.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index d6b2574585..a7a6f6adcf 100755 --- a/src/init.cpp +++ b/src/init.cpp @@ -608,11 +608,10 @@ bool AppInit2(ThreadHandlerPtr threads) { CreatePidFile(GetPidFile(), pid); - // While this is technically successful we need to return false - // in order to shut down the parent process. This can be improved - // by either returning an enum or checking if the current process - // is a child process. - return false; + // Now that we are forked we can request a shutdown so the parent + // exits while the child lives on. + StartShutdown(); + return true; } pid_t sid = setsid(); From c8ef4e992b5ba3b5507f20e5411aa320b33de1c2 Mon Sep 17 00:00:00 2001 From: Paul Jensen Date: Sun, 14 Jan 2018 21:37:52 -0800 Subject: [PATCH 4/9] disable upgrade files --- src/init.cpp | 4 ++-- src/main.cpp | 4 ++-- src/qt/bitcoingui.cpp | 15 ++++++++------- src/rpcrawtransaction.cpp | 5 +++-- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index d6b2574585..e35408c45f 100755 --- a/src/init.cpp +++ b/src/init.cpp @@ -885,7 +885,7 @@ bool AppInit2(ThreadHandlerPtr threads) strErrors << _("Error loading wallet.dat") << "\n"; } - if (GetBoolArg("-upgradewallet", fFirstRun)) +/* if (GetBoolArg("-upgradewallet", fFirstRun)) { int nMaxVersion = GetArg("-upgradewallet", 0); if (nMaxVersion == 0) // the -upgradewallet without argument case @@ -899,7 +899,7 @@ bool AppInit2(ThreadHandlerPtr threads) if (nMaxVersion < pwalletMain->GetVersion()) strErrors << _("Cannot downgrade wallet") << "\n"; pwalletMain->SetMaxVersion(nMaxVersion); - } + }*/ if (fFirstRun) { diff --git a/src/main.cpp b/src/main.cpp index 27db3ee6bb..4f5421ae58 100755 --- a/src/main.cpp +++ b/src/main.cpp @@ -4436,7 +4436,7 @@ void GridcoinServices() if (TimerMain("gather_cpids",480)) msNeuralResponse.clear(); -#ifdef QT_GUI +/*#ifdef QT_GUI // Check for updates once per day. if(GetAdjustedTime() - nLastCheckedForUpdate > 24 * 60 * 60) { @@ -4453,7 +4453,7 @@ void GridcoinServices() } } } -#endif +#endif*/ if (fDebug10) printf(" {/SVC} "); } diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index faea561e2b..9a3406ab62 100755 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -232,13 +232,14 @@ BitcoinGUI::BitcoinGUI(QWidget *parent): rpcConsole = new RPCConsole(this); connect(openRPCConsoleAction, SIGNAL(triggered()), rpcConsole, SLOT(show())); - upgrader = new UpgradeDialog(this); - connect(upgradeAction, SIGNAL(triggered()), upgrader, SLOT(show())); - connect(upgradeAction, SIGNAL(triggered()), upgrader, SLOT(upgrade())); - connect(downloadAction, SIGNAL(triggered()), upgrader, SLOT(show())); - connect(downloadAction, SIGNAL(triggered()), upgrader, SLOT(blocks())); - - diagnosticsDialog = new DiagnosticsDialog(this); + upgrader = new UpgradeDialog(this); + connect(upgradeAction, SIGNAL(triggered()), upgrader, SLOT(show())); + connect(upgradeAction, SIGNAL(triggered()), upgrader, SLOT(upgrade())); + upgradeAction->setEnabled(false); + connect(downloadAction, SIGNAL(triggered()), upgrader, SLOT(show())); + connect(downloadAction, SIGNAL(triggered()), upgrader, SLOT(blocks())); + + diagnosticsDialog = new DiagnosticsDialog(this); // Clicking on "Verify Message" in the address book sends you to the verify message tab diff --git a/src/rpcrawtransaction.cpp b/src/rpcrawtransaction.cpp index 44bf3531dd..a919b4c900 100644 --- a/src/rpcrawtransaction.cpp +++ b/src/rpcrawtransaction.cpp @@ -491,7 +491,8 @@ Value downloadstate(const Array& params, bool fHelp) Value upgrade(const Array& params, bool fHelp) { - if (fHelp || params.size() != 0) + throw runtime_error("upgrader disabled"); + /*if (fHelp || params.size() != 0) throw runtime_error( "upgrade \n" "Upgrades client to the latest version.\n" @@ -517,7 +518,7 @@ Value upgrade(const Array& params, bool fHelp) QMetaObject::invokeMethod(&checker, "check", Qt::QueuedConnection); #endif return "Initiated download of client"; - } + }*/ } From 3464c8186be9f7503d0018d8a16e450a7d5e8b27 Mon Sep 17 00:00:00 2001 From: Paul Jensen Date: Mon, 15 Jan 2018 22:47:05 -0800 Subject: [PATCH 5/9] use setVisible(false) instead of setEnabled(false) --- src/qt/bitcoingui.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 9a3406ab62..181241d5c1 100755 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -235,7 +235,7 @@ BitcoinGUI::BitcoinGUI(QWidget *parent): upgrader = new UpgradeDialog(this); connect(upgradeAction, SIGNAL(triggered()), upgrader, SLOT(show())); connect(upgradeAction, SIGNAL(triggered()), upgrader, SLOT(upgrade())); - upgradeAction->setEnabled(false); + upgradeAction->setVisible(false); connect(downloadAction, SIGNAL(triggered()), upgrader, SLOT(show())); connect(downloadAction, SIGNAL(triggered()), upgrader, SLOT(blocks())); From a85de02ccf6c6a8ff4908813a2ef6767b7585d98 Mon Sep 17 00:00:00 2001 From: Marco Nilsson Date: Fri, 19 Jan 2018 13:50:23 +0100 Subject: [PATCH 6/9] Avoid creating threads on each RPC request. Since threads are never removed from the thread group this caused resource leaks when handling RPC requests. The threading has been removed as this is handled asynchronously by ASIO anyway. This has the added benefit of the requests being faster. When trying getblockbynumber 25 times in a row the operation went down from 3.756s to 2.43s. A speedup of 35%. --- src/bitcoinrpc.cpp | 42 +++++++++++------------------------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/src/bitcoinrpc.cpp b/src/bitcoinrpc.cpp index 1eb6348f5b..1692c13f76 100644 --- a/src/bitcoinrpc.cpp +++ b/src/bitcoinrpc.cpp @@ -42,8 +42,6 @@ static asio::io_service* rpc_io_service = NULL; const Object emptyobj; -void ThreadRPCServer3(void* parg); - static inline unsigned short GetDefaultRPCPort() { return GetBoolArg("-testnet", false) ? 25715 : 15715; @@ -669,6 +667,8 @@ class AcceptedConnectionImpl : public AcceptedConnection iostreams::stream< SSLIOStreamDevice > _stream; }; +void ServiceConnection(AcceptedConnection *conn); + void StopRPCThreads() { printf("Stop RPC IO service\n"); @@ -744,8 +744,7 @@ static void RPCAcceptHandler(boost::shared_ptr< basic_socket_acceptor { // Immediately start accepting new connections, except when we're cancelled or our socket is closed. - if (error != asio::error::operation_aborted - && acceptor->is_open()) + if (error != asio::error::operation_aborted && acceptor->is_open()) RPCListen(acceptor, context, fUseSSL); AcceptedConnectionImpl* tcp_conn = dynamic_cast< AcceptedConnectionImpl* >(conn); @@ -759,18 +758,17 @@ static void RPCAcceptHandler(boost::shared_ptr< basic_socket_acceptor // Restrict callers by IP. It is important to // do this before starting client thread, to filter out // certain DoS and misbehaving clients. - else if (tcp_conn - && !ClientAllowed(tcp_conn->peer.address())) + else if (tcp_conn && !ClientAllowed(tcp_conn->peer.address())) { // Only send a 403 if we're not using SSL to prevent a DoS during the SSL handshake. if (!fUseSSL) conn->stream() << HTTPReply(HTTP_FORBIDDEN, "", false) << std::flush; delete conn; } - - // start HTTP client thread - else if (!netThreads->createThread(ThreadRPCServer3, conn,"ThreadRPCServer3")) { - printf("Failed to create RPC server client thread\r\n"); + else + { + ServiceConnection(conn); + conn->close(); delete conn; } } @@ -985,30 +983,17 @@ static string JSONRPCExecBatch(const Array& vReq) return write_string(Value(ret), false) + "\n"; } -static CCriticalSection cs_THREAD_RPCHANDLER; - -void ThreadRPCServer3(void* parg) +void ServiceConnection(AcceptedConnection *conn) { // Make this thread recognisable as the RPC handler RenameThread("grc-rpchand"); - { - LOCK(cs_THREAD_RPCHANDLER); - } - AcceptedConnection *conn = (AcceptedConnection *) parg; - bool fRun = true; while (true) { if (fShutdown || !fRun) - { - conn->close(); - delete conn; - { - LOCK(cs_THREAD_RPCHANDLER); - } - return; - } + break; + map mapHeaders; string strRequest; @@ -1073,11 +1058,6 @@ void ThreadRPCServer3(void* parg) break; } } - - delete conn; - { - LOCK(cs_THREAD_RPCHANDLER); - } } json_spirit::Value CRPCTable::execute(const std::string &strMethod, const json_spirit::Array ¶ms) const From 12b1ed06058b48638d71fb80d627451636c1fd57 Mon Sep 17 00:00:00 2001 From: Marco Nilsson Date: Sat, 20 Jan 2018 10:40:13 +0100 Subject: [PATCH 7/9] Update changelog for 3.7.4.0. --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bd2a4f45c..d021f76f81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [3.7.4.0] 2018-01-20 +### Fixed + - Fix RPC resource leak regression, #848 (@denravonska). + - Fix incorrect return code when forking, #832 (@denravonska). + +### Removed + - Remove upgrader option until rewritten, #836 (@Foggyx420). + ## [3.7.3.0] 2018-01-13 ### Fixed - Fix for UI getting stuck in splash screen (@denravonska). From de8932cbda994bd6a1194f137db3419c4fa910a6 Mon Sep 17 00:00:00 2001 From: Marco Nilsson Date: Sat, 20 Jan 2018 12:25:38 +0100 Subject: [PATCH 8/9] Add note about RPC calls being faster. --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d021f76f81..8cc6f9bdc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [3.7.4.0] 2018-01-20 ### Fixed - - Fix RPC resource leak regression, #848 (@denravonska). + - Fix RPC resource leak regression. This also reduces RPC overhead, + making calls ~25-35% faster, #848 (@denravonska). - Fix incorrect return code when forking, #832 (@denravonska). ### Removed From a07d3381d05b5d72dcc3484d4f0114b25207b74d Mon Sep 17 00:00:00 2001 From: Marco Nilsson Date: Sat, 20 Jan 2018 13:09:57 +0100 Subject: [PATCH 9/9] Compact RPCAcceptHandler. --- src/bitcoinrpc.cpp | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/bitcoinrpc.cpp b/src/bitcoinrpc.cpp index 1692c13f76..7352591720 100644 --- a/src/bitcoinrpc.cpp +++ b/src/bitcoinrpc.cpp @@ -742,35 +742,30 @@ static void RPCAcceptHandler(boost::shared_ptr< basic_socket_acceptor AcceptedConnection* conn, const boost::system::error_code& error) { - // Immediately start accepting new connections, except when we're cancelled or our socket is closed. if (error != asio::error::operation_aborted && acceptor->is_open()) RPCListen(acceptor, context, fUseSSL); - AcceptedConnectionImpl* tcp_conn = dynamic_cast< AcceptedConnectionImpl* >(conn); - // TODO : Actually handle errors - if (error) - { - delete conn; - } + if (!error) + { + // Restrict callers by IP. It is important to + // do this before starting client thread, to filter out + // certain DoS and misbehaving clients. + AcceptedConnectionImpl* tcp_conn = dynamic_cast< AcceptedConnectionImpl* >(conn); + if (tcp_conn && !ClientAllowed(tcp_conn->peer.address())) + { + // Only send a 403 if we're not using SSL to prevent a DoS during the SSL handshake. + if (!fUseSSL) + conn->stream() << HTTPReply(HTTP_FORBIDDEN, "", false) << std::flush; + } + else + ServiceConnection(conn); - // Restrict callers by IP. It is important to - // do this before starting client thread, to filter out - // certain DoS and misbehaving clients. - else if (tcp_conn && !ClientAllowed(tcp_conn->peer.address())) - { - // Only send a 403 if we're not using SSL to prevent a DoS during the SSL handshake. - if (!fUseSSL) - conn->stream() << HTTPReply(HTTP_FORBIDDEN, "", false) << std::flush; - delete conn; - } - else - { - ServiceConnection(conn); - conn->close(); - delete conn; + conn->close(); } + + delete conn; } void ThreadRPCServer2(void* parg)