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 @@
                       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 @@ def rpc(self, method, args):
             # 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 @@ def estimate_fee_per_kb(self, N):
         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 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 @@ def __init__(self, wallet_service):
         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 @@ def try_to_create_my_orders(self):
         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.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 @@ def __init__(self, wallet):
                 # 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 @@ def update_blockheight(self):
         """
 
         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 @@ def sync_unspent(self):
         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 @@
 #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 @@ def closeEvent(self, event):
             event.accept()
             if self.reactor.threadpool is not None:
                 self.reactor.threadpool.stop()
-            if reactor.running:
-                self.reactor.stop()
+            stop_reactor()
         else:
             event.ignore()