Updater: Directly wait for parent to exit instead of using shell

Also use NSWorkspace to launch applications.

Should hopefully fix the "updater getting stuck running in the
background" behaviour that started in MacOS 27.

src/common/cocoa_tools.h

@@ -46,8 +46,8 @@ std::optional<std::string> GetBundlePath();
 /// Get the bundle path to the actual application without any translocation fun
 std::optional<std::string> GetNonTranslocatedBundlePath();
 
-/// Launch the given application once this one quits
-bool DelayedLaunch(std::string_view file, std::span<const std::string_view> args = {});
+/// Launches an application through LaunchServices, optionally passing command-line arguments.
+bool LaunchApplication(std::string_view path, std::span<const std::string_view> args, Error* error);
 
 /// Returns the size of a NSView in pixels.
 std::optional<std::pair<int, int>> GetViewSizeInPixels(const void* view);

src/common/cocoa_tools.mm

@@ -134,30 +134,58 @@ std::optional<std::string> CocoaTools::GetNonTranslocatedBundlePath()
   return ret;
 }
 
-bool CocoaTools::DelayedLaunch(std::string_view file, std::span<const std::string_view> args)
+bool CocoaTools::LaunchApplication(std::string_view path, std::span<const std::string_view> args, Error* error)
 {
   @autoreleasepool
   {
-    const int pid = [[NSProcessInfo processInfo] processIdentifier];
-
-    // Hopefully we're not too large here...
-    std::string task_args =
-      fmt::format("while /bin/ps -p {} > /dev/null; do /bin/sleep 0.1; done; exec /usr/bin/open \"{}\"", pid, file);
-    if (!args.empty())
+    if (path.empty())
     {
-      task_args += " --args";
+      Error::SetString(error, "Cannot launch an application with an empty path.");
+      return false;
+    }
+
+    NSMutableArray<NSString*>* const launch_args = [NSMutableArray arrayWithCapacity:args.size()];
     for (const std::string_view& arg : args)
+      [launch_args addObject:arg.empty() ? @"" : StringViewToNSString(arg)];
+
+    NSWorkspaceOpenConfiguration* const configuration = [NSWorkspaceOpenConfiguration configuration];
+    [configuration setActivates:YES];
+    [configuration setAllowsRunningApplicationSubstitution:NO];
+    [configuration setCreatesNewApplicationInstance:YES];
+    // Callers present launch failures in their own UI. Asking LaunchServices to present them would result in a
+    // duplicate system alert followed by the DuckStation/Updater error dialog.
+    [configuration setPromptsUserIfNeeded:NO];
+    [configuration setArguments:launch_args];
+
+    // NSWorkspace's completion handler runs on a concurrent queue, so it is safe to wait here. Waiting for the
+    // callback ensures that callers do not exit until LaunchServices has confirmed that the replacement app started.
+    dispatch_semaphore_t const completion_semaphore = dispatch_semaphore_create(0);
+    __block bool launch_succeeded = false;
+    __block NSError* launch_error = nil;
+    [[NSWorkspace sharedWorkspace]
+      openApplicationAtURL:[NSURL fileURLWithPath:StringViewToNSString(path) isDirectory:YES]
+             configuration:configuration
+         completionHandler:^(NSRunningApplication* app, NSError* nserror) {
+           launch_succeeded = (app != nil && nserror == nil);
+           launch_error = [nserror retain];
+           dispatch_semaphore_signal(completion_semaphore);
+         }];
+    dispatch_semaphore_wait(completion_semaphore, DISPATCH_TIME_FOREVER);
+
+    if (!launch_succeeded)
     {
-        task_args += " \"";
-        task_args += arg;
-        task_args += "\"";
-      }
+      if (launch_error)
+        Error::SetStringFmt(error, "Failed to launch '{}': {}", path, NSErrorToString(launch_error));
+      else
+        Error::SetStringFmt(error, "Failed to launch '{}'.", path);
     }
+    [launch_error release];
+
+#if !OS_OBJECT_USE_OBJC
+    dispatch_release(completion_semaphore);
+#endif
 
-    NSTask* task = [NSTask new];
-    [task setExecutableURL:[NSURL fileURLWithPath:@"/bin/sh"]];
-    [task setArguments:@[ @"-c", [NSString stringWithUTF8String:task_args.c_str()] ]];
-    return [task launchAndReturnError:nil];
+    return launch_succeeded;
   }
 }
 

src/duckstation-qt/autoupdaterdialog.cpp

@@ -901,14 +901,21 @@ bool AutoUpdaterDialog::processUpdate(const std::vector<u8>& update_data)
   INFO_LOG("Beginning update:\nUpdater path: {}\nZip path: {}\nStaging directory: {}\nOutput directory: {}",
            updater_app, zip_path, staging_directory, bundle_path.value());
 
+  const std::string parent_process_id = fmt::format("{}", QCoreApplication::applicationPid());
   const std::string_view args[] = {
+    parent_process_id,
     zip_path,
     staging_directory,
     bundle_path.value(),
   };
 
   // Kick off updater!
-  CocoaTools::DelayedLaunch(updater_app, args);
+  if (!CocoaTools::LaunchApplication(updater_app, args, &error))
+  {
+    reportError(fmt::format("Failed to start updater: {}", error.GetDescription()));
+    return false;
+  }
+
   return true;
 }
 

src/updater/cocoa_main.mm

@@ -4,6 +4,8 @@
 #include "cocoa_progress_callback.h"
 #include "updater.h"
 
+#include "common/cocoa_tools.h"
+#include "common/error.h"
 #include "common/file_system.h"
 #include "common/log.h"
 #include "common/path.h"
@@ -11,17 +13,59 @@
 #include "common/string_util.h"
 #include "common/timer.h"
 
+#include <cerrno>
 #include <cstdlib>
+#include <sys/event.h>
+#include <sys/types.h>
 #include <thread>
+#include <unistd.h>
 
-static void LaunchApplication(const char* path)
+static bool WaitForProcessToExit(pid_t process_id, Error* error)
 {
-  @autoreleasepool
+  const int queue = kqueue();
+  if (queue < 0)
   {
-    NSTask* task = [[[NSTask alloc] init] autorelease];
-    [task setLaunchPath:[NSString stringWithUTF8String:path]];
-    [task launch];
+    Error::SetErrno(error, "kqueue() failed: ", errno);
+    return false;
   }
+
+  const ScopedGuard queue_closer = [queue]() { close(queue); };
+
+  struct kevent change;
+  EV_SET(&change, static_cast<uintptr_t>(process_id), EVFILT_PROC, EV_ADD | EV_ENABLE | EV_ONESHOT, NOTE_EXIT, 0,
+         nullptr);
+
+  struct kevent event;
+  int result;
+  do
+  {
+    result = kevent(queue, &change, 1, &event, 1, nullptr);
+  } while (result < 0 && errno == EINTR);
+
+  if (result < 0)
+  {
+    Error::SetErrno(error, "kevent() failed while waiting for DuckStation to exit: ", errno);
+    return false;
+  }
+  if (result == 0)
+  {
+    Error::SetString(error, "kevent() returned without reporting that DuckStation exited.");
+    return false;
+  }
+  if (event.flags & EV_ERROR)
+  {
+    // The process can exit between launching the updater and registering the event. In that case it is safe to
+    // continue. Any other registration error is fatal, because modifying the bundle while DuckStation is still
+    // running could leave the installation unusable.
+    const int event_error = static_cast<int>(event.data);
+    if (event_error == ESRCH)
+      return true;
+
+    Error::SetErrno(error, "Failed to monitor DuckStation: ", event_error);
+    return false;
+  }
+
+  return true;
 }
 
 int main(int argc, char* argv[])
@@ -35,20 +79,22 @@ int main(int argc, char* argv[])
 
   CocoaProgressCallback progress;
 
-  if (argc != 4)
+  if (argc != 5)
   {
-    progress.ModalError("Expected 3 arguments: update zip, staging directory, output directory.\n\nThis program is not "
-                        "intended to be run manually, please use the Qt frontend and click Help->Check for Updates.");
+    progress.ModalError(
+      "Expected 4 arguments: parent process id, update zip, staging directory, output directory.\n\nThis program is "
+      "not intended to be run manually, please use the Qt frontend and click Help->Check for Updates.");
     return EXIT_FAILURE;
   }
 
-  std::string zip_path = argv[1];
-  std::string staging_directory = argv[2];
-  std::string destination_directory = argv[3];
+  const int parent_process_id = StringUtil::FromChars<int>(argv[1]).value_or(0);
+  std::string zip_path = argv[2];
+  std::string staging_directory = argv[3];
+  std::string destination_directory = argv[4];
 
-  if (zip_path.empty() || staging_directory.empty() || destination_directory.empty())
+  if (parent_process_id <= 0 || zip_path.empty() || staging_directory.empty() || destination_directory.empty())
   {
-    progress.ModalError("One or more parameters is empty.");
+    progress.ModalError("One or more parameters is invalid.");
     return EXIT_FAILURE;
   }
 
@@ -59,10 +105,10 @@ int main(int argc, char* argv[])
     Log::SetFileOutputParams(true, log_path.c_str());
   }
 
-  std::string program_to_launch = Path::Combine(destination_directory, "Contents/MacOS/DuckStation");
+  std::string application_to_launch = destination_directory;
   int result = EXIT_SUCCESS;
 
-  std::thread worker([&progress, zip_path = std::move(zip_path),
+  std::thread worker([&progress, parent_process_id, zip_path = std::move(zip_path),
                       destination_directory = std::move(destination_directory),
                       staging_directory = std::move(staging_directory), &result]() {
     ScopedGuard app_stopper([]() {
@@ -85,6 +131,15 @@ int main(int argc, char* argv[])
       });
     });
 
+    progress.FormatStatusText("Waiting for DuckStation process {} to exit...", parent_process_id);
+    Error wait_error;
+    if (!WaitForProcessToExit(static_cast<pid_t>(parent_process_id), &wait_error))
+    {
+      progress.FormatModalError("Failed to wait for DuckStation to exit: {}", wait_error.GetDescription());
+      result = EXIT_FAILURE;
+      return;
+    }
+
     Updater updater(&progress);
     if (!updater.Initialize(std::move(staging_directory), std::move(destination_directory)))
     {
@@ -142,8 +197,15 @@ int main(int argc, char* argv[])
 
   if (result == EXIT_SUCCESS)
   {
-    progress.FormatInformation("Launching '{}'...", program_to_launch);
-    LaunchApplication(program_to_launch.c_str());
+    progress.FormatInformation("Launching '{}'...", application_to_launch);
+    Error launch_error;
+    if (!CocoaTools::LaunchApplication(application_to_launch, {}, &launch_error))
+    {
+      progress.FormatModalError("The update was installed successfully, but DuckStation could not be restarted: {}\n\n"
+                                "Please launch DuckStation manually.",
+                                launch_error.GetDescription());
+      result = EXIT_FAILURE;
+    }
   }
 
   return result;