From 5fc67ec65a4d89cd2b0864c55612cc2ac89fdb86 Mon Sep 17 00:00:00 2001 From: woodser Date: Sat, 18 Nov 2023 14:36:42 -0500 Subject: [PATCH] fix app hanging after connection change with parallel handling --- .../api/CoreMoneroConnectionsService.java | 105 ++++++++++-------- .../main/java/haveno/core/trade/Trade.java | 6 +- .../core/xmr/wallet/XmrWalletService.java | 93 ++++++++-------- 3 files changed, 107 insertions(+), 97 deletions(-) diff --git a/core/src/main/java/haveno/core/api/CoreMoneroConnectionsService.java b/core/src/main/java/haveno/core/api/CoreMoneroConnectionsService.java index 38109c345f..a98bcf6237 100644 --- a/core/src/main/java/haveno/core/api/CoreMoneroConnectionsService.java +++ b/core/src/main/java/haveno/core/api/CoreMoneroConnectionsService.java @@ -53,6 +53,7 @@ public final class CoreMoneroConnectionsService { private final Object lock = new Object(); + private final Object listenersLock = new Object(); private final Config config; private final CoreContext coreContext; private final Preferences preferences; @@ -146,7 +147,7 @@ public final class CoreMoneroConnectionsService { } public void addConnectionListener(MoneroConnectionManagerListener listener) { - synchronized (lock) { + synchronized (listenersLock) { listeners.add(listener); } } @@ -512,9 +513,11 @@ public final class CoreMoneroConnectionsService { } updatePolling(); - // notify listeners - synchronized (lock) { - for (MoneroConnectionManagerListener listener : listeners) listener.onConnectionChanged(currentConnection); + // notify listeners in parallel + synchronized (listenersLock) { + for (MoneroConnectionManagerListener listener : listeners) { + new Thread(() -> listener.onConnectionChanged(currentConnection)).start(); + } } } @@ -537,64 +540,68 @@ public final class CoreMoneroConnectionsService { private void stopPolling() { synchronized (lock) { - if (daemonPollLooper != null) daemonPollLooper.stop(); + if (daemonPollLooper != null) { + daemonPollLooper.stop(); + daemonPollLooper = null; + } } } private void pollDaemonInfo() { - if (isShutDownStarted) return; - try { - log.debug("Polling daemon info"); - if (daemon == null) throw new RuntimeException("No daemon connection"); - synchronized (this) { + synchronized (lock) { + if (isShutDownStarted) return; + try { + log.debug("Polling daemon info"); + if (daemon == null) throw new RuntimeException("No daemon connection"); lastInfo = daemon.getInfo(); - } - chainHeight.set(lastInfo.getTargetHeight() == 0 ? lastInfo.getHeight() : lastInfo.getTargetHeight()); + chainHeight.set(lastInfo.getTargetHeight() == 0 ? lastInfo.getHeight() : lastInfo.getTargetHeight()); - // set peer connections - // TODO: peers often uknown due to restricted RPC call, skipping call to get peer connections - // try { - // peers.set(getOnlinePeers()); - // } catch (Exception err) { - // // TODO: peers unknown due to restricted RPC call - // } - // numPeers.set(peers.get().size()); - numPeers.set(lastInfo.getNumOutgoingConnections() + lastInfo.getNumIncomingConnections()); - peers.set(new ArrayList()); - - // handle error recovery - if (lastErrorTimestamp != null) { - log.info("Successfully fetched daemon info after previous error"); - lastErrorTimestamp = null; - } + // set peer connections + // TODO: peers often uknown due to restricted RPC call, skipping call to get peer connections + // try { + // peers.set(getOnlinePeers()); + // } catch (Exception err) { + // // TODO: peers unknown due to restricted RPC call + // } + // numPeers.set(peers.get().size()); + numPeers.set(lastInfo.getNumOutgoingConnections() + lastInfo.getNumIncomingConnections()); + peers.set(new ArrayList()); + + // handle error recovery + if (lastErrorTimestamp != null) { + log.info("Successfully fetched daemon info after previous error"); + lastErrorTimestamp = null; + } - // update and notify connected state - if (!Boolean.TRUE.equals(connectionManager.isConnected())) { - connectionManager.checkConnection(); - } + // update and notify connected state + if (!Boolean.TRUE.equals(connectionManager.isConnected())) { + connectionManager.checkConnection(); + } - // clear error message - if (Boolean.TRUE.equals(connectionManager.isConnected()) && HavenoUtils.havenoSetup != null) { - HavenoUtils.havenoSetup.getWalletServiceErrorMsg().set(null); - } - } catch (Exception e) { + // clear error message + if (Boolean.TRUE.equals(connectionManager.isConnected()) && HavenoUtils.havenoSetup != null) { + HavenoUtils.havenoSetup.getWalletServiceErrorMsg().set(null); + } + } catch (Exception e) { - // log error message periodically - if ((lastErrorTimestamp == null || System.currentTimeMillis() - lastErrorTimestamp > MIN_ERROR_LOG_PERIOD_MS)) { - lastErrorTimestamp = System.currentTimeMillis(); - log.warn("Could not update daemon info: " + e.getMessage()); - if (DevEnv.isDevMode()) e.printStackTrace(); - } + // skip if shut down or connected + if (isShutDownStarted || Boolean.TRUE.equals(isConnected())) return; + + // log error message periodically + if ((lastErrorTimestamp == null || System.currentTimeMillis() - lastErrorTimestamp > MIN_ERROR_LOG_PERIOD_MS)) { + lastErrorTimestamp = System.currentTimeMillis(); + log.warn("Could not update daemon info: " + e.getMessage()); + if (DevEnv.isDevMode()) e.printStackTrace(); + } - // check connection which notifies of changes - synchronized (this) { + // check connection which notifies of changes if (connectionManager.getAutoSwitch()) connectionManager.setConnection(connectionManager.getBestAvailableConnection()); else connectionManager.checkConnection(); - } - // set error message - if (!Boolean.TRUE.equals(connectionManager.isConnected()) && HavenoUtils.havenoSetup != null) { - HavenoUtils.havenoSetup.getWalletServiceErrorMsg().set(e.getMessage()); + // set error message + if (!Boolean.TRUE.equals(connectionManager.isConnected()) && HavenoUtils.havenoSetup != null) { + HavenoUtils.havenoSetup.getWalletServiceErrorMsg().set(e.getMessage()); + } } } } diff --git a/core/src/main/java/haveno/core/trade/Trade.java b/core/src/main/java/haveno/core/trade/Trade.java index d423a0b46d..c5c8b85f4a 100644 --- a/core/src/main/java/haveno/core/trade/Trade.java +++ b/core/src/main/java/haveno/core/trade/Trade.java @@ -587,8 +587,10 @@ public abstract class Trade implements Tradable, Model { getArbitrator().setPubKeyRing(arbitrator.getPubKeyRing()); }); - // listen to daemon connection - xmrWalletService.getConnectionsService().addConnectionListener(newConnection -> onConnectionChanged(newConnection)); + // handle daemon changes with max parallelization + xmrWalletService.getConnectionsService().addConnectionListener(newConnection -> { + HavenoUtils.submitTask(() -> onConnectionChanged(newConnection)); + }); // check if done if (isPayoutUnlocked()) { diff --git a/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java b/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java index d20d28c072..c4250d0816 100644 --- a/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java +++ b/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java @@ -3,7 +3,6 @@ package haveno.core.xmr.wallet; import com.google.common.util.concurrent.Service.State; import com.google.inject.name.Named; -import common.utils.GenUtils; import common.utils.JsonUtils; import haveno.common.UserThread; import haveno.common.config.Config; @@ -97,6 +96,7 @@ public class XmrWalletService { private static final String ADDRESS_FILE_POSTFIX = ".address.txt"; private static final int NUM_MAX_BACKUP_WALLETS = 1; private static final int MONERO_LOG_LEVEL = 0; + private static final int MAX_SYNC_ATTEMPTS = 3; private static final boolean PRINT_STACK_TRACE = false; private final Preferences preferences; @@ -651,14 +651,20 @@ public class XmrWalletService { private void initialize() { + // set and listen to daemon connection + connectionsService.addConnectionListener(newConnection -> { + onConnectionChanged(newConnection); + }); + // initialize main wallet if connected or previously created maybeInitMainWallet(true); - - // set and listen to daemon connection - connectionsService.addConnectionListener(newConnection -> onConnectionChanged(newConnection)); } private void maybeInitMainWallet(boolean sync) { + maybeInitMainWallet(sync, MAX_SYNC_ATTEMPTS); + } + + private void maybeInitMainWallet(boolean sync, int numAttempts) { synchronized (walletLock) { // open or create wallet main wallet @@ -676,52 +682,47 @@ public class XmrWalletService { // sync wallet and register listener if (wallet != null) { log.info("Monero wallet uri={}, path={}", wallet.getRpcConnection().getUri(), wallet.getPath()); - if (sync) { - int maxAttempts = 3; - for (int i = 0; i < maxAttempts; i++) { - try { - - // sync main wallet - log.info("Syncing main wallet"); - long time = System.currentTimeMillis(); - wallet.sync(); // blocking - wasWalletSynced = true; - log.info("Done syncing main wallet in " + (System.currentTimeMillis() - time) + " ms"); - wallet.startSyncing(connectionsService.getRefreshPeriodMs()); - if (getMoneroNetworkType() != MoneroNetworkType.MAINNET) log.info("Monero wallet balance={}, unlocked balance={}", wallet.getBalance(0), wallet.getUnlockedBalance(0)); - - // reapply connection after wallet synced - onConnectionChanged(connectionsService.getConnection()); + if (sync && numAttempts > 0) { + try { + + // sync main wallet + log.info("Syncing main wallet"); + long time = System.currentTimeMillis(); + wallet.sync(); // blocking + wasWalletSynced = true; + log.info("Done syncing main wallet in " + (System.currentTimeMillis() - time) + " ms"); + wallet.startSyncing(connectionsService.getRefreshPeriodMs()); + if (getMoneroNetworkType() != MoneroNetworkType.MAINNET) log.info("Monero wallet balance={}, unlocked balance={}", wallet.getBalance(0), wallet.getUnlockedBalance(0)); + + // reapply connection after wallet synced + onConnectionChanged(connectionsService.getConnection()); - // TODO: using this to signify both daemon and wallet synced, use separate sync handlers? - connectionsService.doneDownload(); - - // notify setup that main wallet is initialized - // TODO: app fully initializes after this is set to true, even though wallet might not be initialized if unconnected. wallet will be created when connection detected - // refactor startup to call this and sync off main thread? but the calls to e.g. getBalance() fail with 'wallet and network is not yet initialized' + // TODO: using this to signify both daemon and wallet synced, use separate sync handlers? + connectionsService.doneDownload(); + + // notify setup that main wallet is initialized + // TODO: app fully initializes after this is set to true, even though wallet might not be initialized if unconnected. wallet will be created when connection detected + // refactor startup to call this and sync off main thread? but the calls to e.g. getBalance() fail with 'wallet and network is not yet initialized' + HavenoUtils.havenoSetup.getWalletInitialized().set(true); + + // save but skip backup on initialization + saveMainWallet(false); + } catch (Exception e) { + log.warn("Error syncing main wallet: {}", e.getMessage()); + if (numAttempts <= 1) { + log.warn("Failed to sync main wallet. Opening app without syncing", numAttempts); HavenoUtils.havenoSetup.getWalletInitialized().set(true); - - // save but skip backup on initialization saveMainWallet(false); - break; - } catch (Exception e) { - log.warn("Error syncing main wallet: {}", e.getMessage()); - if (i == maxAttempts - 1) { - log.warn("Failed to sync main wallet after {} attempts. Opening app without syncing", maxAttempts); - HavenoUtils.havenoSetup.getWalletInitialized().set(true); - saveMainWallet(false); - // reschedule to init main wallet - UserThread.runAfter(() -> { - new Thread(() -> { - log.warn("Restarting attempts to sync main wallet"); - maybeInitMainWallet(true); - }); - }, connectionsService.getRefreshPeriodMs() / 1000); - } else { - log.warn("Trying again in {} seconds", connectionsService.getRefreshPeriodMs() / 1000); - GenUtils.waitFor(connectionsService.getRefreshPeriodMs()); - } + // reschedule to init main wallet + UserThread.runAfter(() -> { + new Thread(() -> maybeInitMainWallet(true, MAX_SYNC_ATTEMPTS)).start(); + }, connectionsService.getRefreshPeriodMs() / 1000); + } else { + log.warn("Trying again in {} seconds", connectionsService.getRefreshPeriodMs() / 1000); + UserThread.runAfter(() -> { + new Thread(() -> maybeInitMainWallet(true, numAttempts - 1)).start(); + }, connectionsService.getRefreshPeriodMs() / 1000); } } }