VPN-7042: Fix shutdown/upgrade deadlock (#10520)

* Prevent multiple emissions of Controller::readyToQuit
* Allow synchronous completion of TaskControllerAction
* Only terminate GUI process with pkill during postinstall

Author: oskirby

macos/pkg/scripts/postinstall.in

@@ -35,7 +35,7 @@ exec 2>&1 > $LOG_DIR/postinstall.log
 echo "Running postinstall at $(date)"
 echo "Installing Mozilla VPN to ${APP_DIR}"
 
-pkill -x "Mozilla VPN" || echo "Unable to kill GUI, not running?"
+pkill -U "${USER}" -x "Mozilla VPN" || echo "Unable to kill GUI, not running?"
 sleep 1
 
 ## Before MacOS 13, take extra care to prevent tampering with the app.
@@ -62,8 +62,11 @@ echo "Codesign successful!"
 
 # Unload the daemon
 UPDATING=
-if launchctl bootout system/${CODESIGN_APP_IDENTIFIER}.daemon 2>/dev/null; then
-  echo "Update detected"
+if launchctl bootout system/${CODESIGN_APP_IDENTIFIER}.service 2>/dev/null; then
+  echo "Update detected (SMAppService)"
+  UPDATING=1
+elif launchctl bootout system/${CODESIGN_APP_IDENTIFIER}.daemon 2>/dev/null; then
+  echo "Update detected (legacy daemon)"
   UPDATING=1
 fi
 if [ -f "$DAEMON_PLIST_PATH" ]; then

src/controller.cpp

@@ -249,16 +249,12 @@ void Controller::quit() {
   logger.debug() << "Quitting";
 
   if (!isActive()) {
-    m_nextStep = Quit;
     emit readyToQuit();
     return;
   }
 
   m_nextStep = Quit;
-
-  if (m_state == StateOn || m_state == StateOnPartial ||
-      m_state == StateSwitching || m_state == StateSilentSwitching ||
-      m_state == StateConnecting || m_state == StateConfirming) {
+  if (m_state > StateDisconnecting) {
     deactivate();
     return;
   }

src/tasks/controlleraction/taskcontrolleraction.cpp

@@ -42,12 +42,13 @@ void TaskControllerAction::run() {
   Q_ASSERT(controller);
 
   connect(controller, &Controller::stateChanged, this,
-          &TaskControllerAction::stateChanged, Qt::QueuedConnection);
+          &TaskControllerAction::stateChanged);
 
-  bool expectSignal = false;
+  // Give the action some time to complete.
+  m_timer.start(TASKCONTROLLER_TIMER_MSEC);
 
+  bool expectSignal = false;
   m_lastState = controller->state();
-
   switch (m_action) {
     case eActivate:
       expectSignal = controller->activate(m_serverData);
@@ -70,15 +71,10 @@ void TaskControllerAction::run() {
       break;
   }
 
-  // No signal expected. Probably, the VPN is already in the right state. Let's
-  // use the timer to wait 1 cycle only.
+  // If no signal is expected, the VPN is probably already in the right state.
   if (!expectSignal) {
-    m_timer.start(0);
-    return;
+    checkStatus();
   }
-
-  // Fallback 3 seconds.
-  m_timer.start(TASKCONTROLLER_TIMER_MSEC);
 }
 
 void TaskControllerAction::stateChanged() {