From 77dd8ca5d7bfa4e6c41a8f790fc9e32eee8c61c2 Mon Sep 17 00:00:00 2001 From: Craig Raw Date: Thu, 20 Aug 2020 11:25:34 +0200 Subject: [PATCH] unregister closed tab and dialog subscribers, fix connection error reset behaviour --- .../sparrowwallet/sparrow/AppController.java | 16 ++++++++++++ .../sparrowwallet/sparrow/WalletTabData.java | 4 +++ .../sparrow/control/DeviceAddressDialog.java | 4 ++- .../sparrow/control/DeviceSignDialog.java | 4 ++- .../sparrow/control/WalletExportDialog.java | 4 ++- .../sparrow/control/WalletImportDialog.java | 4 ++- .../event/TransactionTabsClosedEvent.java | 17 +++++++++++++ .../sparrow/event/WalletTabsClosedEvent.java | 17 +++++++++++++ .../keystoreimport/KeystoreImportDialog.java | 4 +++ .../sparrow/net/ElectrumServer.java | 25 ++++++++++++------- .../sparrow/net/TcpTransport.java | 14 +++++++++-- .../transaction/HeadersController.java | 5 ++++ .../sparrow/transaction/InputController.java | 5 ++++ .../sparrow/transaction/InputsController.java | 5 ++++ .../sparrow/transaction/OutputController.java | 5 ++++ .../transaction/OutputsController.java | 5 ++++ .../transaction/TransactionController.java | 10 ++++++++ .../sparrow/transaction/TransactionForm.java | 4 +++ .../TransactionFormController.java | 15 +++++++++++ .../sparrow/wallet/TransactionEntry.java | 11 ++++++++ .../sparrow/wallet/WalletForm.java | 10 ++++++++ .../sparrow/wallet/WalletFormController.java | 15 +++++++++++ 22 files changed, 188 insertions(+), 15 deletions(-) create mode 100644 src/main/java/com/sparrowwallet/sparrow/event/TransactionTabsClosedEvent.java create mode 100644 src/main/java/com/sparrowwallet/sparrow/event/WalletTabsClosedEvent.java diff --git a/src/main/java/com/sparrowwallet/sparrow/AppController.java b/src/main/java/com/sparrowwallet/sparrow/AppController.java index 2109aa98..3e728f39 100644 --- a/src/main/java/com/sparrowwallet/sparrow/AppController.java +++ b/src/main/java/com/sparrowwallet/sparrow/AppController.java @@ -188,6 +188,18 @@ public class AppController implements Initializable { EventManager.get().post(new OpenWalletsEvent(getOpenWallets())); } + List closedWalletTabs = c.getRemoved().stream().map(tab -> (TabData)tab.getUserData()) + .filter(tabData -> tabData.getType() == TabData.TabType.WALLET).map(tabData -> (WalletTabData)tabData).collect(Collectors.toList()); + if(!closedWalletTabs.isEmpty()) { + EventManager.get().post(new WalletTabsClosedEvent(closedWalletTabs)); + } + + List closedTransactionTabs = c.getRemoved().stream().map(tab -> (TabData)tab.getUserData()) + .filter(tabData -> tabData.getType() == TabData.TabType.TRANSACTION).map(tabData -> (TransactionTabData)tabData).collect(Collectors.toList()); + if(!closedTransactionTabs.isEmpty()) { + EventManager.get().post(new TransactionTabsClosedEvent(closedTransactionTabs)); + } + if(tabs.getTabs().isEmpty()) { Stage tabStage = (Stage)tabs.getScene().getWindow(); tabStage.setTitle("Sparrow"); @@ -268,10 +280,14 @@ public class AppController implements Initializable { } }); connectionService.setOnFailed(failEvent -> { + //Close connection here to create a new transport next time we try + connectionService.resetConnection(); + changeMode = false; onlineProperty.setValue(false); changeMode = true; + log.debug("Connection failed", failEvent.getSource().getException()); EventManager.get().post(new ConnectionFailedEvent(failEvent.getSource().getException())); }); diff --git a/src/main/java/com/sparrowwallet/sparrow/WalletTabData.java b/src/main/java/com/sparrowwallet/sparrow/WalletTabData.java index d8a48441..6b2a4f66 100644 --- a/src/main/java/com/sparrowwallet/sparrow/WalletTabData.java +++ b/src/main/java/com/sparrowwallet/sparrow/WalletTabData.java @@ -12,6 +12,10 @@ public class WalletTabData extends TabData { this.walletForm = walletForm; } + public WalletForm getWalletForm() { + return walletForm; + } + public Wallet getWallet() { return walletForm.getWallet(); } diff --git a/src/main/java/com/sparrowwallet/sparrow/control/DeviceAddressDialog.java b/src/main/java/com/sparrowwallet/sparrow/control/DeviceAddressDialog.java index e43cc063..8f64f1b4 100644 --- a/src/main/java/com/sparrowwallet/sparrow/control/DeviceAddressDialog.java +++ b/src/main/java/com/sparrowwallet/sparrow/control/DeviceAddressDialog.java @@ -19,6 +19,9 @@ public class DeviceAddressDialog extends DeviceDialog { this.keyDerivation = keyDerivation; EventManager.get().register(this); + setOnCloseRequest(event -> { + EventManager.get().unregister(this); + }); } @Override @@ -28,7 +31,6 @@ public class DeviceAddressDialog extends DeviceDialog { @Subscribe public void addressDisplayed(AddressDisplayedEvent event) { - EventManager.get().unregister(this); setResult(event.getAddress()); this.close(); } diff --git a/src/main/java/com/sparrowwallet/sparrow/control/DeviceSignDialog.java b/src/main/java/com/sparrowwallet/sparrow/control/DeviceSignDialog.java index f8857dc7..043f5b93 100644 --- a/src/main/java/com/sparrowwallet/sparrow/control/DeviceSignDialog.java +++ b/src/main/java/com/sparrowwallet/sparrow/control/DeviceSignDialog.java @@ -15,6 +15,9 @@ public class DeviceSignDialog extends DeviceDialog { super(devices); this.psbt = psbt; EventManager.get().register(this); + setOnCloseRequest(event -> { + EventManager.get().unregister(this); + }); setResultConverter(dialogButton -> dialogButton.getButtonData().isCancelButton() ? null : psbt); } @@ -26,7 +29,6 @@ public class DeviceSignDialog extends DeviceDialog { @Subscribe public void psbtSigned(PSBTSignedEvent event) { if(psbt == event.getPsbt()) { - EventManager.get().unregister(this); setResult(event.getSignedPsbt()); this.close(); } diff --git a/src/main/java/com/sparrowwallet/sparrow/control/WalletExportDialog.java b/src/main/java/com/sparrowwallet/sparrow/control/WalletExportDialog.java index 4bcde41d..73a86e89 100644 --- a/src/main/java/com/sparrowwallet/sparrow/control/WalletExportDialog.java +++ b/src/main/java/com/sparrowwallet/sparrow/control/WalletExportDialog.java @@ -21,6 +21,9 @@ public class WalletExportDialog extends Dialog { public WalletExportDialog(Wallet wallet) { EventManager.get().register(this); + setOnCloseRequest(event -> { + EventManager.get().unregister(this); + }); final DialogPane dialogPane = getDialogPane(); @@ -64,7 +67,6 @@ public class WalletExportDialog extends Dialog { @Subscribe public void walletExported(WalletExportEvent event) { - EventManager.get().unregister(this); wallet = event.getWallet(); setResult(wallet); this.close(); diff --git a/src/main/java/com/sparrowwallet/sparrow/control/WalletImportDialog.java b/src/main/java/com/sparrowwallet/sparrow/control/WalletImportDialog.java index f7de69cf..698c53ae 100644 --- a/src/main/java/com/sparrowwallet/sparrow/control/WalletImportDialog.java +++ b/src/main/java/com/sparrowwallet/sparrow/control/WalletImportDialog.java @@ -16,6 +16,9 @@ public class WalletImportDialog extends Dialog { public WalletImportDialog() { EventManager.get().register(this); + setOnCloseRequest(event -> { + EventManager.get().unregister(this); + }); final DialogPane dialogPane = getDialogPane(); @@ -51,7 +54,6 @@ public class WalletImportDialog extends Dialog { @Subscribe public void walletImported(WalletImportEvent event) { - EventManager.get().unregister(this); wallet = event.getWallet(); setResult(wallet); this.close(); diff --git a/src/main/java/com/sparrowwallet/sparrow/event/TransactionTabsClosedEvent.java b/src/main/java/com/sparrowwallet/sparrow/event/TransactionTabsClosedEvent.java new file mode 100644 index 00000000..f0c8c49f --- /dev/null +++ b/src/main/java/com/sparrowwallet/sparrow/event/TransactionTabsClosedEvent.java @@ -0,0 +1,17 @@ +package com.sparrowwallet.sparrow.event; + +import com.sparrowwallet.sparrow.TransactionTabData; + +import java.util.List; + +public class TransactionTabsClosedEvent { + private final List closedTransactionTabData; + + public TransactionTabsClosedEvent(List closedTransactionTabData) { + this.closedTransactionTabData = closedTransactionTabData; + } + + public List getClosedTransactionTabData() { + return closedTransactionTabData; + } +} diff --git a/src/main/java/com/sparrowwallet/sparrow/event/WalletTabsClosedEvent.java b/src/main/java/com/sparrowwallet/sparrow/event/WalletTabsClosedEvent.java new file mode 100644 index 00000000..77233e24 --- /dev/null +++ b/src/main/java/com/sparrowwallet/sparrow/event/WalletTabsClosedEvent.java @@ -0,0 +1,17 @@ +package com.sparrowwallet.sparrow.event; + +import com.sparrowwallet.sparrow.WalletTabData; + +import java.util.List; + +public class WalletTabsClosedEvent { + private final List closedWalletTabData; + + public WalletTabsClosedEvent(List closedWalletTabData) { + this.closedWalletTabData = closedWalletTabData; + } + + public List getClosedWalletTabData() { + return closedWalletTabData; + } +} diff --git a/src/main/java/com/sparrowwallet/sparrow/keystoreimport/KeystoreImportDialog.java b/src/main/java/com/sparrowwallet/sparrow/keystoreimport/KeystoreImportDialog.java index 274efa70..fbce4020 100644 --- a/src/main/java/com/sparrowwallet/sparrow/keystoreimport/KeystoreImportDialog.java +++ b/src/main/java/com/sparrowwallet/sparrow/keystoreimport/KeystoreImportDialog.java @@ -27,6 +27,10 @@ public class KeystoreImportDialog extends Dialog { public KeystoreImportDialog(Wallet wallet, KeystoreSource initialSource) { EventManager.get().register(this); + setOnCloseRequest(event -> { + EventManager.get().unregister(this); + }); + final DialogPane dialogPane = getDialogPane(); try { diff --git a/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java b/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java index f4ba8921..52bc535a 100644 --- a/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java +++ b/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java @@ -579,7 +579,6 @@ public class ElectrumServer { private final boolean subscribe; private boolean firstCall = true; private Thread reader; - private Throwable lastReaderException; private long feeRatesRetrievedAt; public ConnectionService() { @@ -598,7 +597,7 @@ public class ElectrumServer { if(firstCall) { electrumServer.connect(); - reader = new Thread(new ReadRunnable()); + reader = new Thread(new ReadRunnable(), "ElectrumServerReadThread"); reader.setDaemon(true); reader.setUncaughtExceptionHandler(ConnectionService.this); reader.start(); @@ -639,8 +638,7 @@ public class ElectrumServer { return new FeeRatesUpdatedEvent(blockTargetFeeRates); } } else { - firstCall = true; - throw new ServerException("Connection to server failed", lastReaderException); + resetConnection(); } } @@ -649,12 +647,21 @@ public class ElectrumServer { }; } + public void resetConnection() { + try { + closeActiveConnection(); + firstCall = true; + } catch (ServerException e) { + log.error("Error closing connection during connection reset", e); + } + } + @Override public boolean cancel() { try { closeActiveConnection(); } catch (ServerException e) { - log.error("Eror closing connection", e); + log.error("Error closing connection", e); } return super.cancel(); @@ -664,12 +671,11 @@ public class ElectrumServer { public void reset() { super.reset(); firstCall = true; - lastReaderException = null; } @Override public void uncaughtException(Thread t, Throwable e) { - this.lastReaderException = e; + log.error("Uncaught error in ConnectionService", e); } } @@ -679,8 +685,9 @@ public class ElectrumServer { try { TcpTransport tcpTransport = (TcpTransport)getTransport(); tcpTransport.readInputLoop(); - } catch (ServerException e) { - throw new RuntimeException(e.getCause() != null ? e.getCause() : e); + } catch(ServerException e) { + //Only debug logging here as the exception has been passed on to the ConnectionService thread via TcpTransport + log.debug("Read thread terminated", e); } } } diff --git a/src/main/java/com/sparrowwallet/sparrow/net/TcpTransport.java b/src/main/java/com/sparrowwallet/sparrow/net/TcpTransport.java index 00b512e9..4bb700fc 100644 --- a/src/main/java/com/sparrowwallet/sparrow/net/TcpTransport.java +++ b/src/main/java/com/sparrowwallet/sparrow/net/TcpTransport.java @@ -28,6 +28,8 @@ public class TcpTransport implements Transport, Closeable { private final JsonRpcServer jsonRpcServer = new JsonRpcServer(); private final SubscriptionService subscriptionService = new SubscriptionService(); + private Exception lastException; + public TcpTransport(HostAndPort server) { this.server = server; this.socketFactory = SocketFactory.getDefault(); @@ -50,7 +52,7 @@ public class TcpTransport implements Transport, Closeable { out.flush(); } - private synchronized String readResponse() { + private synchronized String readResponse() throws IOException { while(reading) { try { wait(); @@ -60,6 +62,10 @@ public class TcpTransport implements Transport, Closeable { } } + if(lastException != null) { + throw new IOException("Error reading response", lastException); + } + reading = true; notifyAll(); @@ -83,8 +89,12 @@ public class TcpTransport implements Transport, Closeable { } catch(InterruptedException e) { //Restore interrupt status and continue Thread.currentThread().interrupt(); - } catch(IOException e) { + } catch(Exception e) { if(running) { + lastException = e; + reading = false; + notifyAll(); + //Allow this thread to terminate as we will need to reconnect with a new transport anyway throw new ServerException(e); } } diff --git a/src/main/java/com/sparrowwallet/sparrow/transaction/HeadersController.java b/src/main/java/com/sparrowwallet/sparrow/transaction/HeadersController.java index ba391090..10527850 100644 --- a/src/main/java/com/sparrowwallet/sparrow/transaction/HeadersController.java +++ b/src/main/java/com/sparrowwallet/sparrow/transaction/HeadersController.java @@ -196,6 +196,11 @@ public class HeadersController extends TransactionFormController implements Init initializeView(); } + @Override + protected TransactionForm getTransactionForm() { + return headersForm; + } + private void initializeView() { Transaction tx = headersForm.getTransaction(); diff --git a/src/main/java/com/sparrowwallet/sparrow/transaction/InputController.java b/src/main/java/com/sparrowwallet/sparrow/transaction/InputController.java index 5c370588..cc660195 100644 --- a/src/main/java/com/sparrowwallet/sparrow/transaction/InputController.java +++ b/src/main/java/com/sparrowwallet/sparrow/transaction/InputController.java @@ -461,6 +461,11 @@ public class InputController extends TransactionFormController implements Initia initializeView(); } + @Override + protected TransactionForm getTransactionForm() { + return inputForm; + } + @Override protected String describeScriptChunk(ScriptChunk chunk) { String chunkString = super.describeScriptChunk(chunk); diff --git a/src/main/java/com/sparrowwallet/sparrow/transaction/InputsController.java b/src/main/java/com/sparrowwallet/sparrow/transaction/InputsController.java index aee9fd49..92d1f7b4 100644 --- a/src/main/java/com/sparrowwallet/sparrow/transaction/InputsController.java +++ b/src/main/java/com/sparrowwallet/sparrow/transaction/InputsController.java @@ -49,6 +49,11 @@ public class InputsController extends TransactionFormController implements Initi initialiseView(); } + @Override + protected TransactionForm getTransactionForm() { + return inputsForm; + } + private void initialiseView() { Transaction tx = inputsForm.getTransaction(); count.setText(Integer.toString(tx.getInputs().size())); diff --git a/src/main/java/com/sparrowwallet/sparrow/transaction/OutputController.java b/src/main/java/com/sparrowwallet/sparrow/transaction/OutputController.java index 1e83d2ad..704b74c8 100644 --- a/src/main/java/com/sparrowwallet/sparrow/transaction/OutputController.java +++ b/src/main/java/com/sparrowwallet/sparrow/transaction/OutputController.java @@ -147,6 +147,11 @@ public class OutputController extends TransactionFormController implements Initi initializeView(); } + @Override + protected TransactionForm getTransactionForm() { + return outputForm; + } + @Subscribe public void blockTransactionOutputsFetched(BlockTransactionOutputsFetchedEvent event) { if(event.getTxId().equals(outputForm.getTransaction().getTxId()) && outputForm.getPsbt() == null && outputForm.getIndex() >= event.getPageStart() && outputForm.getIndex() < event.getPageEnd()) { diff --git a/src/main/java/com/sparrowwallet/sparrow/transaction/OutputsController.java b/src/main/java/com/sparrowwallet/sparrow/transaction/OutputsController.java index ef54bd22..6ee637c0 100644 --- a/src/main/java/com/sparrowwallet/sparrow/transaction/OutputsController.java +++ b/src/main/java/com/sparrowwallet/sparrow/transaction/OutputsController.java @@ -51,6 +51,11 @@ public class OutputsController extends TransactionFormController implements Init } } + @Override + protected TransactionForm getTransactionForm() { + return outputsForm; + } + @Subscribe public void bitcoinUnitChanged(BitcoinUnitChangedEvent event) { total.refresh(event.getBitcoinUnit()); diff --git a/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionController.java b/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionController.java index 7525596a..396cb6a4 100644 --- a/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionController.java +++ b/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionController.java @@ -8,6 +8,7 @@ import com.sparrowwallet.drongo.psbt.PSBTOutput; import com.sparrowwallet.drongo.wallet.BlockTransaction; import com.sparrowwallet.sparrow.AppController; import com.sparrowwallet.sparrow.EventManager; +import com.sparrowwallet.sparrow.TransactionTabData; import com.sparrowwallet.sparrow.control.TransactionHexArea; import com.sparrowwallet.sparrow.event.*; import com.sparrowwallet.sparrow.net.ElectrumServer; @@ -489,4 +490,13 @@ public class TransactionController implements Initializable { highlightTxHex(); } } + + @Subscribe + public void transactionTabsClosed(TransactionTabsClosedEvent event) { + for(TransactionTabData tabData : event.getClosedTransactionTabData()) { + if(tabData.getTransactionData() == txdata) { + EventManager.get().unregister(this); + } + } + } } \ No newline at end of file diff --git a/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionForm.java b/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionForm.java index dd04b2dc..93a342f0 100644 --- a/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionForm.java +++ b/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionForm.java @@ -23,6 +23,10 @@ public abstract class TransactionForm { this.txdata = txdata; } + public TransactionData getTransactionData() { + return txdata; + } + public Transaction getTransaction() { return txdata.getTransaction(); } diff --git a/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionFormController.java b/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionFormController.java index 89a3d668..714d4ab7 100644 --- a/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionFormController.java +++ b/src/main/java/com/sparrowwallet/sparrow/transaction/TransactionFormController.java @@ -1,9 +1,13 @@ package com.sparrowwallet.sparrow.transaction; +import com.google.common.eventbus.Subscribe; import com.sparrowwallet.drongo.address.Address; import com.sparrowwallet.drongo.protocol.NonStandardScriptException; import com.sparrowwallet.drongo.protocol.TransactionOutput; import com.sparrowwallet.sparrow.BaseController; +import com.sparrowwallet.sparrow.EventManager; +import com.sparrowwallet.sparrow.TransactionTabData; +import com.sparrowwallet.sparrow.event.TransactionTabsClosedEvent; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.scene.chart.PieChart; @@ -18,6 +22,8 @@ import java.util.List; public abstract class TransactionFormController extends BaseController { private static final int MAX_PIE_SEGMENTS = 200; + protected abstract TransactionForm getTransactionForm(); + protected void addPieData(PieChart pie, List outputs) { ObservableList outputsPieData = FXCollections.observableArrayList(); @@ -64,6 +70,15 @@ public abstract class TransactionFormController extends BaseController { }); } + @Subscribe + public void transactionTabsClosed(TransactionTabsClosedEvent event) { + for(TransactionTabData tabData : event.getClosedTransactionTabData()) { + if(tabData.getTransactionData() == getTransactionForm().getTransactionData()) { + EventManager.get().unregister(this); + } + } + } + public static class TransactionReferenceContextMenu extends ContextMenu { public TransactionReferenceContextMenu(String reference) { MenuItem referenceItem = new MenuItem("Copy Reference"); diff --git a/src/main/java/com/sparrowwallet/sparrow/wallet/TransactionEntry.java b/src/main/java/com/sparrowwallet/sparrow/wallet/TransactionEntry.java index 1a01a14b..bf903ea4 100644 --- a/src/main/java/com/sparrowwallet/sparrow/wallet/TransactionEntry.java +++ b/src/main/java/com/sparrowwallet/sparrow/wallet/TransactionEntry.java @@ -7,8 +7,10 @@ import com.sparrowwallet.drongo.wallet.BlockTransactionHash; import com.sparrowwallet.drongo.wallet.BlockTransactionHashIndex; import com.sparrowwallet.drongo.wallet.Wallet; import com.sparrowwallet.sparrow.EventManager; +import com.sparrowwallet.sparrow.WalletTabData; import com.sparrowwallet.sparrow.event.WalletBlockHeightChangedEvent; import com.sparrowwallet.sparrow.event.WalletEntryLabelChangedEvent; +import com.sparrowwallet.sparrow.event.WalletTabsClosedEvent; import javafx.beans.property.IntegerProperty; import javafx.beans.property.IntegerPropertyBase; import javafx.beans.property.LongProperty; @@ -204,4 +206,13 @@ public class TransactionEntry extends Entry implements Comparable