Merge branch 'only-disconnect-on-quit-button'

5 files changed, 32 insertions, 46 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 327cbd5a41..1267a6822e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -42,6 +42,8 @@ Line wrap the file at 100 chars.                                              Th
 - Reorganize settings into more logical categories.
 - Upgrade wireguard-go to 20220703234212 (Windows: v0.5.3).
 - Prune bridges far away from the selected relay.
+- Stay connected when desktop app is killed or crashes. The only situation where the app now
+  disconnects on quit is when the user presses the quit button.
 
 #### Windows
 - Remove dependency on `ipconfig.exe`. Call `DnsFlushResolverCache` to flush the DNS cache.
diff --git a/dist-assets/linux/before-install.sh b/dist-assets/linux/before-install.sh
index 6f076b129e..a8efb02adb 100644
--- a/dist-assets/linux/before-install.sh
+++ b/dist-assets/linux/before-install.sh
@@ -11,6 +11,8 @@ if which systemctl &> /dev/null; then
     fi
 fi
 
+# This can be removed when 2022.4 is unsupported. That version is the last version where
+# before-remove.sh doesn't kill the GUI on upgrade.
 pkill -x "mullvad-gui" || true
 
 rm -f /var/cache/mullvad-vpn/relays.json
diff --git a/dist-assets/linux/before-remove.sh b/dist-assets/linux/before-remove.sh
index 340cd272cc..38fcb0364f 100644
--- a/dist-assets/linux/before-remove.sh
+++ b/dist-assets/linux/before-remove.sh
@@ -1,6 +1,8 @@
 #!/usr/bin/env bash
 set -eu
 
+pkill -x "mullvad-gui" || true
+
 is_number_re='^[0-9]+$'
 # Check if we're running during an upgrade step on Fedora
 # https://fedoraproject.org/wiki/Packaging:Scriptlets#Syntax
@@ -18,7 +20,5 @@ fi
 systemctl stop mullvad-daemon.service || true
 systemctl disable mullvad-daemon.service || true
 
-pkill -x "mullvad-gui" || true
-
 /opt/Mullvad\ VPN/resources/mullvad-setup reset-firewall || echo "Failed to reset firewall"
 /opt/Mullvad\ VPN/resources/mullvad-setup remove-device || echo "Failed to remove device from account"
diff --git a/dist-assets/windows/installer.nsh b/dist-assets/windows/installer.nsh
index b699982ad0..7b1f552e2e 100644
--- a/dist-assets/windows/installer.nsh
+++ b/dist-assets/windows/installer.nsh
@@ -1155,7 +1155,7 @@
 
 	Pop $FullUninstall
 
-	nsExec::Exec '"$INSTDIR\Mullvad VPN.exe" --quit-without-disconnect' $0
+	nsExec::Exec `taskkill /t /im "${APP_EXECUTABLE_FILENAME}"` $0
 	Sleep 500
 	nsExec::Exec `taskkill /f /t /im "${APP_EXECUTABLE_FILENAME}"` $0
 
diff --git a/gui/src/main/index.ts b/gui/src/main/index.ts
index 99aaaf5a8b..cba52d61e1 100644
--- a/gui/src/main/index.ts
+++ b/gui/src/main/index.ts
@@ -55,7 +55,6 @@ const linuxSplitTunneling = process.platform === 'linux' && require('./linux-spl
 const windowsSplitTunneling = process.platform === 'win32' && require('./windows-split-tunneling');
 
 enum CommandLineOptions {
-  quitWithoutDisconnect = '--quit-without-disconnect',
   showChanges = '--show-changes',
   disableResetNavigation = '--disable-reset-navigation', // development only
 }
@@ -109,8 +108,6 @@ class ApplicationMain
 
   private macOsScrollbarVisibility?: MacOsScrollbarVisibility;
 
-  private stayConnectedOnQuit = false;
-
   private changelog?: IChangelog;
 
   private navigationHistory?: IHistoryObject;
@@ -128,13 +125,9 @@ class ApplicationMain
 
     this.overrideAppPaths();
 
-    // This ensures that only a single instance is running at the same time, but also exits if
-    // there's no already running instance when the quit without disconnect flag is supplied.
-    if (
-      !app.requestSingleInstanceLock() ||
-      process.argv.includes(CommandLineOptions.quitWithoutDisconnect)
-    ) {
-      this.quitWithoutDisconnect();
+    // This ensures that only a single instance is running at the same time.
+    if (!app.requestSingleInstanceLock()) {
+      app.quit();
       return;
     }
 
@@ -168,7 +161,7 @@ class ApplicationMain
       log.error(
         `Render process exited with exit code ${details.exitCode} due to ${details.reason}`,
       );
-      this.quitWithoutDisconnect();
+      app.quit();
     });
     app.on('child-process-gone', (_event, details) => {
       log.error(
@@ -223,14 +216,8 @@ class ApplicationMain
   };
 
   private addSecondInstanceEventHandler() {
-    app.on('second-instance', (_event, argv, _workingDirectory) => {
-      if (argv.includes(CommandLineOptions.quitWithoutDisconnect)) {
-        // Quit if another instance is started with the quit without disconnect flag.
-        this.quitWithoutDisconnect();
-      } else {
-        // If no action was provided to the new instance the window is opened.
-        this.userInterface?.showWindow();
-      }
+    app.on('second-instance', (_event, _argv, _workingDirectory) => {
+      this.userInterface?.showWindow();
     });
   }
 
@@ -287,20 +274,31 @@ class ApplicationMain
 
   private onActivate = () => this.userInterface?.showWindow();
 
-  private quitWithoutDisconnect() {
-    this.stayConnectedOnQuit = true;
+  private async disconnectAndQuit() {
+    if (this.daemonRpc.isConnected) {
+      try {
+        await this.daemonRpc.disconnectTunnel();
+        log.info('Disconnected the tunnel');
+      } catch (e) {
+        const error = e as Error;
+        log.error(`Failed to disconnect the tunnel: ${error.message}`);
+      }
+    } else {
+      log.info('Cannot close the tunnel because there is no active connection to daemon.');
+    }
+
     app.quit();
   }
 
   // This is a last try to disconnect and quit gracefully if the app quits without having received
   // the before-quit event.
-  private onQuit = async () => {
+  private onQuit = () => {
     if (this.quitStage !== AppQuitStage.ready) {
-      await this.prepareToQuit();
+      this.prepareToQuit();
     }
   };
 
-  private onBeforeQuit = async (event: Electron.Event) => {
+  private onBeforeQuit = (event: Electron.Event) => {
     switch (this.quitStage) {
       case AppQuitStage.unready:
         // postpone the app shutdown
@@ -310,7 +308,7 @@ class ApplicationMain
 
         log.info('Quit initiated');
 
-        await this.prepareToQuit();
+        this.prepareToQuit();
 
         // terminate the app
         this.quitStage = AppQuitStage.ready;
@@ -328,23 +326,7 @@ class ApplicationMain
     }
   };
 
-  private async prepareToQuit() {
-    if (this.stayConnectedOnQuit) {
-      log.info('Not disconnecting tunnel on quit');
-    } else {
-      if (this.daemonRpc.isConnected) {
-        try {
-          await this.daemonRpc.disconnectTunnel();
-          log.info('Disconnected the tunnel');
-        } catch (e) {
-          const error = e as Error;
-          log.error(`Failed to disconnect the tunnel: ${error.message}`);
-        }
-      } else {
-        log.info('Cannot close the tunnel because there is no active connection to daemon.');
-      }
-    }
-
+  private prepareToQuit() {
     // Unsubscribe the event handler
     try {
       if (this.daemonEventListener) {
@@ -781,7 +763,7 @@ class ApplicationMain
       },
     );
 
-    IpcMainEventChannel.app.handleQuit(() => app.quit());
+    IpcMainEventChannel.app.handleQuit(() => this.disconnectAndQuit());
     IpcMainEventChannel.app.handleOpenUrl(async (url) => {
       if (Object.values(config.links).find((link) => url.startsWith(link))) {
         await shell.openExternal(url);