stop a terminal server test left with back or done, and leave services stopped when tor starts after going offline
What the developer wrote
Authored by Craig Raw
stop a terminal server test left with back or done, and leave services stopped when tor starts after going offline
What changed, and why it matters
This commit fixes two reliability bugs in the Sparrow Wallet desktop app. First, it prevents a server-connection test from continuing to run in the background after the user leaves the test screen, which could otherwise connect to the wrong server or interfere with later settings. Second, it stops the wallet from automatically restarting network services when the Tor privacy tool comes back online if the user has intentionally gone offline. These are stability and privacy fixes rather than remote-hack vulnerabilities.
Reviewers should verify that no other service success/failure handlers call restartServices() unconditionally, and that all dialog paths that can leave ServerTestDialog set abandoned=true and cancel active services. No immediate user action is required beyond updating to a release containing this commit.
Security signals we found
Unintended background network connection to previously or newly configured server after dialog abandonment
Automatic service restart on Tor readiness despite user-initiated offline state
Stale event-manager registration of a cancelled connection service
Cross-thread state flag (volatile abandoned) for GUI/application thread coordination
Evidence from the diff
The patch addresses two asynchronous lifecycle issues. In AppServices.java, the Tor service success handler now only calls restartServices() when the application is online or configured for online mode, preventing unwanted service restart after going offline. In ServerTestDialog.java, an ‘abandoned’ flag is introduced to suppress Tor and Electrum connection startup when the user navigates back or presses Done before the test completes. It also centralizes connection cancellation into cancelConnectionService() and explicitly unregisters the connection service from the event manager when a cancel takes effect, avoiding leaked/stale service handlers.
Changed components
com.sparrowwallet.sparrow.AppServicescom.sparrowwallet.sparrow.terminal.settings.ServerTestDialogTorService lifecycleElectrumServer.ConnectionService lifecycleEventManager registrationInspect captured patch +25 / −9
### src/main/java/com/sparrowwallet/sparrow/AppServices.java
@@ -479,7 +479,9 @@ private TorService createTorService() {
torService.setOnSucceeded(workerStateEvent -> {
Tor.setDefault(torService.getValue());
torService.cancel();
- restartServices();
+ if(onlineProperty.get() || Config.get().getMode() == Mode.ONLINE) {
+ restartServices();
+ }
EventManager.get().post(new TorReadyStatusEvent());
});
torService.setOnFailed(workerStateEvent -> {
### src/main/java/com/sparrowwallet/sparrow/terminal/settings/ServerTestDialog.java
@@ -28,6 +28,7 @@ public class ServerTestDialog extends DialogWindow {
private TorService torService;
private ElectrumServer.ConnectionService connectionService;
+ private volatile boolean abandoned; //written on the gui thread and read on the application thread
public ServerTestDialog() {
super("Server Test");
@@ -65,8 +66,13 @@ public ServerTestDialog() {
}
public void onBack() {
+ //Going back abandons the test, so one still running is not left to complete against whatever server is configured by then
+ abandoned = true;
+ EventManager.get().unregister(this);
close();
+ Platform.runLater(this::cancelConnectionService);
+
if(Config.get().getServerType() == ServerType.PUBLIC_ELECTRUM_SERVER) {
PublicElectrumDialog publicElectrumServer = new PublicElectrumDialog();
publicElectrumServer.showDialog(SparrowTerminal.get().getGui());
@@ -92,13 +98,13 @@ public void onTest() {
}
public void onDone() {
+ //A test not yet started is not started once the dialog is gone: it would cancel the connection requested below
+ abandoned = true;
EventManager.get().unregister(this);
close();
Platform.runLater(() -> {
- if(connectionService != null && connectionService.isRunning()) {
- connectionService.cancel();
- }
+ cancelConnectionService();
if(Config.get().getMode() == Mode.ONLINE && !(AppServices.isConnecting() || AppServices.isConnected())) {
EventManager.get().post(new RequestConnectEvent());
}
@@ -117,8 +123,10 @@ private void startTor() {
torService.setOnSucceeded(workerStateEvent -> {
Tor.setDefault(torService.getValue());
torService.cancel();
- appendText("\nTor running, connecting to " + Config.get().getServer().getUrl() + "...");
- startElectrumConnection();
+ if(!abandoned) {
+ appendText("\nTor running, connecting to " + Config.get().getServer().getUrl() + "...");
+ startElectrumConnection();
+ }
});
torService.setOnFailed(workerStateEvent -> {
torService.cancel();
@@ -129,10 +137,16 @@ private void startTor() {
torService.start();
}
- private void startElectrumConnection() {
- if(connectionService != null && connectionService.isRunning()) {
- connectionService.cancel();
+ //A cancel that takes effect runs neither of the handlers that unregister the service, so it is unregistered here. One that comes after the test
+ //has finished does not take effect, and the handler already on its way unregisters the service itself
+ private void cancelConnectionService() {
+ if(connectionService != null && connectionService.isRunning() && connectionService.cancel()) {
+ EventManager.get().unregister(connectionService);
}
+ }
+
+ private void startElectrumConnection() {
+ cancelConnectionService();
AppServices.cancelConnection();
Why this scored 34/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.