Fixes #673. Shutdown cleanly on failure to access blockheight

Prior to this commit, in case an RPC failure occurred when
accesing the block height, the program would continue but the
wallet would be in an un-writeable state (for command line
programs, specifically yield generators; for Qt the shutdown
would occur).
This commit slightly cleans up the process of shutting down,
ensuring that duplicate shutdown calls do not result in
stack traces. It also ensures that also for command line
programs, the application will immediately shutdown if the
regular heartbeat call to query the block height fails, as this
risks inconsistencies in the wallet (though the previous
situation luckily did not result in this as the call to
BaseWallet.close() resulted in the wallet being read only).
A future PR should develop a more sophisticated approach to
RPC call failures that may allow the program to wait.

stopservice

jmbase/jmbase/__init__.py

@@ -7,6 +7,7 @@ from .support import (get_log, chunks, debug_silence, jmprint,
                       utxo_to_utxostr, EXIT_ARGERROR, EXIT_FAILURE,
                       EXIT_SUCCESS, hexbin, dictchanger, listchanger,
                       JM_WALLET_NAME_PREFIX, JM_APP_NAME)
+from .twisted_utils import stop_reactor
 from .bytesprod import BytesProducer
 from .commands import *
 

jmbase/jmbase/twisted_utils.py

@@ -0,0 +1,18 @@
+
+from twisted.internet.error import ReactorNotRunning, AlreadyCancelled
+from twisted.internet import reactor
+
+def stop_reactor():
+    """ Both in startup and shutdown,
+    the value of the bool `reactor.running`
+    does not reliably tell us whether the
+    reactor is running (!). There are startup
+    and shutdown phases not reported externally
+    by IReactorCore.
+    Hence the Exception catch is needed here.
+    """
+    try:
+        if reactor.running:
+            reactor.stop()
+    except ReactorNotRunning:
+        pass

jmclient/jmclient/blockchaininterface.py

@@ -214,9 +214,11 @@ class BitcoinCoreInterface(BlockchainInterface):
             # BareException type).
             log.error("Failure of RPC connection to Bitcoin Core. "
                       "Application cannot continue, shutting down.")
-            if reactor.running:
-                reactor.stop()
+            stop_reactor()
             return None
+        # note that JsonRpcError is not caught here; for some calls, we
+        # have specific behaviour requirements depending on these errors,
+        # so this is handled elsewhere in BitcoinCoreInterface.
         return res
 
     def is_address_labeled(self, utxo, walletname):
@@ -430,7 +432,13 @@ class BitcoinCoreInterface(BlockchainInterface):
         return retval
 
     def get_current_block_height(self):
-        return self.rpc("getblockcount", [])
+        try:
+            res = self.rpc("getblockcount", [])
+        except JsonRpcError as e:
+            log.error("Getblockcount RPC failed with: %i, %s" % (
+                e.code, e.message))
+            res = None
+        return res
 
     def get_best_block_hash(self):
         return self.rpc('getbestblockhash', [])

jmclient/jmclient/maker.py

@@ -6,7 +6,7 @@ import sys
 import abc
 
 import jmbitcoin as btc
-from jmbase import bintohex, hexbin, get_log, EXIT_SUCCESS, EXIT_FAILURE
+from jmbase import bintohex, hexbin, get_log, EXIT_SUCCESS, EXIT_FAILURE, stop_reactor
 from jmclient.wallet import estimate_tx_fee, compute_tx_locktime
 from jmclient.wallet_service import WalletService
 from jmclient.configure import jm_single
@@ -25,7 +25,7 @@ class Maker(object):
         self.nextoid = -1
         self.offerlist = None
         self.sync_wait_loop = task.LoopingCall(self.try_to_create_my_orders)
-        self.sync_wait_loop.start(2.0)
+        self.sync_wait_loop.start(2.0, now=False)
         self.aborted = False
 
     def try_to_create_my_orders(self):
@@ -41,7 +41,7 @@ class Maker(object):
         self.sync_wait_loop.stop()
         if not self.offerlist:
             jlog.info("Failed to create offers, giving up.")
-            sys.exit(EXIT_FAILURE)
+            stop_reactor()
         jlog.info('offerlist={}'.format(self.offerlist))
 
     @hexbin

jmclient/jmclient/wallet_service.py

@@ -15,6 +15,7 @@ from jmclient.output import fmt_tx_data
 from jmclient.blockchaininterface import (INF_HEIGHT, BitcoinCoreInterface,
     BitcoinCoreNoHistoryInterface)
 from jmclient.wallet import FidelityBondMixin
+from jmbase import stop_reactor
 from jmbase.support import jmprint, EXIT_SUCCESS, utxo_to_utxostr, hextobin
 
 
@@ -56,8 +57,10 @@ class WalletService(Service):
                 # a functioning blockchain interface, but
                 # that bci is now failing when we are starting
                 # the wallet service.
-                raise Exception("WalletService failed to start "
-                                "due to inability to query block height.")
+                jlog.error("Failure of RPC connection to Bitcoin Core in "
+                          "wallet service startup. Application cannot "
+                          "continue, shutting down.")
+                stop_reactor()
         else:
             jlog.warning("No blockchain source available, " +
                 "wallet tools will not show correct balances.")
@@ -91,8 +94,13 @@ class WalletService(Service):
         """
 
         def critical_error():
-            jlog.error("Failure to get blockheight from Bitcoin Core.")
+            jlog.error("Critical error updating blockheight.")
+            # this cleanup (a) closes the wallet, removing the lock
+            # and (b) signals to clients that the service is no longer
+            # in a running state, both of which can be useful
+            # post reactor shutdown.
             self.stopService()
+            stop_reactor()
             return False
 
         if self.current_blockheight:
@@ -707,6 +715,10 @@ class WalletService(Service):
         st = time.time()
         # block height needs to be real time for addition to our utxos:
         current_blockheight = self.bci.get_current_block_height()
+        if not current_blockheight:
+            # this failure will shut down the application elsewhere, here
+            # just give up:
+            return
         wallet_name = self.get_wallet_name()
         self.reset_utxos()
 

scripts/joinmarket-qt.py

@@ -63,7 +63,7 @@ donation_address_url = "https://bitcoinprivacy.me/joinmarket-donations"
 #Version of this Qt script specifically
 JM_GUI_VERSION = '16dev'
 
-from jmbase import get_log
+from jmbase import get_log, stop_reactor
 from jmbase.support import DUST_THRESHOLD, EXIT_FAILURE, utxo_to_utxostr,\
     bintohex, hextobin, JM_CORE_VERSION
 from jmclient import load_program_config, get_network, update_persist_config,\
@@ -1489,8 +1489,7 @@ class JMMainWindow(QMainWindow):
             event.accept()
             if self.reactor.threadpool is not None:
                 self.reactor.threadpool.stop()
-            if reactor.running:
-                self.reactor.stop()
+            stop_reactor()
         else:
             event.ignore()