From 58ba7a3d88494912063ba8caa4e4992ee43549d4 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Wed, 20 May 2020 07:24:56 -0400
Subject: [PATCH] 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
---
 src/httprpc.cpp              |  4 ----
 src/httpserver.cpp           |  6 +++---
 test/functional/rpc_users.py | 29 ++++++++++++++++-------------
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/src/httprpc.cpp b/src/httprpc.cpp
index bb975a4ea1..3fcf4101f2 100644
--- a/src/httprpc.cpp
+++ b/src/httprpc.cpp
@@ -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 {
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index d001a9690c..110ff95939 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -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);
diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py
index fcaa3b0526..de4169134e 100755
--- a/test/functional/rpc_users.py
+++ b/test/functional/rpc_users.py
@@ -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()