From a2aafd254d07d97166ed4f3a549238612b316dd2 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Thu, 1 Oct 2020 12:58:04 +0100 Subject: [PATCH] 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 | 1 + jmbase/jmbase/twisted_utils.py | 18 ++++++++++++++++++ jmclient/jmclient/blockchaininterface.py | 14 +++++++++++--- jmclient/jmclient/maker.py | 6 +++--- jmclient/jmclient/wallet_service.py | 18 +++++++++++++++--- scripts/joinmarket-qt.py | 5 ++--- 6 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 jmbase/jmbase/twisted_utils.py diff --git a/jmbase/jmbase/__init__.py b/jmbase/jmbase/__init__.py index e49ea0a7d..e071f1118 100644 --- a/jmbase/jmbase/__init__.py +++ b/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 * diff --git a/jmbase/jmbase/twisted_utils.py b/jmbase/jmbase/twisted_utils.py new file mode 100644 index 000000000..57a65847c --- /dev/null +++ b/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 diff --git a/jmclient/jmclient/blockchaininterface.py b/jmclient/jmclient/blockchaininterface.py index 3060f2ba2..dcf52742d 100644 --- a/jmclient/jmclient/blockchaininterface.py +++ b/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', []) diff --git a/jmclient/jmclient/maker.py b/jmclient/jmclient/maker.py index 7d7677f37..598eaa1a3 100644 --- a/jmclient/jmclient/maker.py +++ b/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 diff --git a/jmclient/jmclient/wallet_service.py b/jmclient/jmclient/wallet_service.py index b76eaa8dd..66014aef4 100644 --- a/jmclient/jmclient/wallet_service.py +++ b/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() diff --git a/scripts/joinmarket-qt.py b/scripts/joinmarket-qt.py index 589b15188..edb6e694b 100755 --- a/scripts/joinmarket-qt.py +++ b/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()