Merge #19006: rpc: Avoid crash when g_thread_http was never started

faf45d1f1f997c316fc4c611a23c4456533eefe9 http: Avoid crash when g_thread_http was never started (MarcoFalke)
fa12a37b27f0570a551b8c103ea6537ee4a8e399 test: Replace inline-comments with logs, pep8 formatting (MarcoFalke)
fa83b39ff3ae3fbad93df002915c0e5f99c104a9 init: Remove confusing and redundant InitError (MarcoFalke)

Pull request description:

  Avoid a crash during shutdown when the init sequence failed for some reason

ACKs for top commit:
  promag:
    Tested ACK faf45d1f1f997c316fc4c611a23c4456533eefe9.
  ryanofsky:
    Code review ACK faf45d1f1f997c316fc4c611a23c4456533eefe9. Thanks for updates, this is much easier to parse for me now. Since previous reviews: split out and reverted some cleanups & replaced chmod with mkdir in test
  hebasto:
    ACK faf45d1f1f997c316fc4c611a23c4456533eefe9, tested on Linux Mint 19.3 with the following patch:

Tree-SHA512: 59632bf01c999e65c724e2728ac103250ccd8b0b16fac19d3a2a82639ab73e4f2efb86c78e63c588a5954625d8d0cf9545e2a7e070e6e15d2a54beeb50e00b61
This commit is contained in:
MarcoFalke 2020-05-20 07:24:56 -04:00 committed by Vijay Das Manikpuri
parent ed096a5d28
commit 58ba7a3d88
No known key found for this signature in database
GPG Key ID: DB1D81B01DB7C46E
3 changed files with 19 additions and 20 deletions

View File

@ -9,7 +9,6 @@
#include <httpserver.h>
#include <rpc/protocol.h>
#include <rpc/server.h>
#include <ui_interface.h>
#include <util/strencodings.h>
#include <util/system.h>
#include <util/translation.h>
@ -253,9 +252,6 @@ static bool InitRPCAuthentication()
{
LogPrintf("Using random cookie authentication.\n");
if (!GenerateAuthCookie(&strRPCUserColonPass)) {
uiInterface.ThreadSafeMessageBox(
_("Error: A fatal internal error occurred, see debug.log for details"), // Same message as AbortNode
"", CClientUIInterface::MSG_ERROR);
return false;
}
} else {

View File

@ -423,7 +423,7 @@ bool UpdateHTTPServerLogging(bool enable) {
#endif
}
static std::thread threadHTTP;
static std::thread g_thread_http;
static std::vector<std::thread> g_thread_http_workers;
void StartHTTPServer()
@ -431,7 +431,7 @@ void StartHTTPServer()
LogPrint(BCLog::HTTP, "Starting HTTP server\n");
int rpcThreads = std::max((long)gArgs.GetArg("-rpcthreads", DEFAULT_HTTP_THREADS), 1L);
LogPrintf("HTTP: starting %d worker threads\n", rpcThreads);
threadHTTP = std::thread(ThreadHTTP, eventBase);
g_thread_http = std::thread(ThreadHTTP, eventBase);
for (int i = 0; i < rpcThreads; i++) {
g_thread_http_workers.emplace_back(HTTPWorkQueueRun, g_work_queue.get(), i);
@ -468,7 +468,7 @@ void StopHTTPServer()
boundSockets.clear();
if (eventBase) {
LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n");
threadHTTP.join();
if (g_thread_http.joinable()) g_thread_http.join();
}
if (eventHTTP) {
evhttp_free(eventHTTP);

View File

@ -44,18 +44,15 @@ class HTTPBasicsTest(BitcoinTestFramework):
self.password = lines[3]
with open(os.path.join(get_datadir_path(self.options.tmpdir, 0), "dash.conf"), 'a', encoding='utf8') as f:
f.write(rpcauth+"\n")
f.write(rpcauth2+"\n")
f.write(rpcauth3+"\n")
f.write(rpcauth + "\n")
f.write(rpcauth2 + "\n")
f.write(rpcauth3 + "\n")
with open(os.path.join(get_datadir_path(self.options.tmpdir, 1), "dash.conf"), 'a', encoding='utf8') as f:
f.write(rpcuser+"\n")
f.write(rpcpassword+"\n")
f.write(rpcuser + "\n")
f.write(rpcpassword + "\n")
def run_test(self):
##################################################
# Check correctness of the rpcauth config option #
##################################################
self.log.info('Check correctness of the rpcauth config option')
url = urllib.parse.urlparse(self.nodes[0].url)
#Old authpair
@ -161,9 +158,7 @@ class HTTPBasicsTest(BitcoinTestFramework):
assert_equal(resp.status, 401)
conn.close()
###############################################################
# Check correctness of the rpcuser/rpcpassword config options #
###############################################################
self.log.info('Check correctness of the rpcuser/rpcpassword config options')
url = urllib.parse.urlparse(self.nodes[1].url)
# rpcuser and rpcpassword authpair
@ -203,5 +198,13 @@ class HTTPBasicsTest(BitcoinTestFramework):
conn.close()
self.log.info('Check that failure to write cookie file will abort the node gracefully')
self.stop_node(0)
cookie_file = os.path.join(get_datadir_path(self.options.tmpdir, 0), self.chain, '.cookie.tmp')
os.mkdir(cookie_file)
init_error = 'Error: Unable to start HTTP server. See debug log for details.'
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error)
if __name__ == '__main__':
HTTPBasicsTest ().main ()
HTTPBasicsTest().main()