manager: improved connection handling

QEMU, QMP manager, and SPICE manager each needs to connect. To simplify things
we set a 30s timeout on each one with 1s per try. They will independently try
to connect and if any fail, then an error message will be shown.

This process should be improved for better failure handling.

Managers/UTMInputOutput.h

@@ -24,7 +24,7 @@ NS_ASSUME_NONNULL_BEGIN
 @protocol UTMInputOutput <NSObject>
 
 - (BOOL)startWithError:(NSError **)err;
-- (void)connectWithCompletion: (void(^)(BOOL, NSError* _Nullable)) block;
+- (void)connectWithCompletion:(void(^)(BOOL, NSString * _Nullable))block;
 - (void)disconnect;
 - (void)setDebugMode: (BOOL)debugMode;
 - (UTMScreenshot* _Nullable)screenshot;

Managers/UTMPortAllocator.m

@@ -32,7 +32,7 @@ static NSInteger firstZeroBit(UInt64 x) {
     return (t < 64) ? t : -1;
 }
 
-static BOOL isPortAvailable(NSInteger port) {
+BOOL isPortAvailable(NSInteger port) {
     struct sockaddr_in addr = {0};
     int sockfd = socket(AF_INET, SOCK_STREAM, 0);
     if (sockfd < 0) {

Managers/UTMQemu.m

@@ -153,8 +153,6 @@
 }
 
 - (void)startQemuRemote:(nonnull NSString *)name completion:(void(^)(BOOL,NSString *))completion {
-    dispatch_semaphore_t lock = dispatch_semaphore_create(0);
-    __block NSString *taskIdentifier;
     NSError *error;
     NSData *libBookmark = [self.libraryURL bookmarkDataWithOptions:0
                                     includingResourceValuesForKeys:nil
@@ -166,20 +164,7 @@
     }
     [[_connection remoteObjectProxyWithErrorHandler:^(NSError * _Nonnull error) {
         completion(NO, error.localizedDescription);
-        dispatch_semaphore_signal(lock);
-    }] startQemu:name libraryBookmark:libBookmark argv:self.argv onStarted:^(NSString *identifier) {
-        UTMLog(@"started %@!", identifier);
-        taskIdentifier = identifier;
-        dispatch_semaphore_signal(lock);
-    }];
-    dispatch_semaphore_wait(lock, DISPATCH_TIME_FOREVER);
-    if (!taskIdentifier) {
-        completion(NO, NSLocalizedString(@"Failed to start QEMU process.", @"UTMQemu"));
-    } else {
-        [[_connection remoteObjectProxyWithErrorHandler:^(NSError * _Nonnull error) {
-            completion(NO, error.localizedDescription);
-        }] registerExitHandlerForIdentifier:taskIdentifier handler:completion];
-    }
+    }] startQemu:name libraryBookmark:libBookmark argv:self.argv onExit:completion];
 }
 
 - (void)start:(nonnull NSString *)name completion:(void(^)(BOOL,NSString *))completion {

Managers/UTMSpiceIO.m

@@ -22,12 +22,13 @@
 #import "UTMViewState.h"
 #import "CocoaSpice.h"
 
+extern BOOL isPortAvailable(NSInteger port); // from UTMPortAllocator
 extern NSString *const kUTMErrorDomain;
-static const int kMaxConnectionTries = 10; // qemu needs to start spice server first
+static const int kMaxConnectionTries = 30; // qemu needs to start spice server first
 static const int64_t kRetryWait = (int64_t)1*NSEC_PER_SEC;
 
 typedef void (^doConnect_t)(void);
-typedef void (^connectionCallback_t)(BOOL success, NSError * _Nullable err);
+typedef void (^connectionCallback_t)(BOOL success, NSString * _Nullable msg);
 
 @interface UTMSpiceIO ()
 
@@ -102,21 +103,28 @@ typedef void (^connectionCallback_t)(BOOL success, NSError * _Nullable err);
     return YES;
 }
 
-- (void)connectWithCompletion:(void(^)(BOOL, NSError*))block {
+- (void)connectWithCompletion:(void(^)(BOOL, NSString * _Nullable))block {
     __block int tries = kMaxConnectionTries;
     __block __weak doConnect_t weakDoConnect;
     __weak UTMSpiceIO *weakSelf = self;
     self.doConnect = ^{
         if (tries-- > 0) {
-            if ([weakSelf.spiceConnection connect]) {
-                weakSelf.connectionCallback = block;
+            if (!isPortAvailable(weakSelf.port)) { // port is in use, try connecting
+                if ([weakSelf.spiceConnection connect]) {
+                    weakSelf.connectionCallback = block;
+                    weakSelf.doConnect = nil;
+                    return;
+                }
             } else {
-                dispatch_after(kRetryWait, dispatch_get_global_queue(QOS_CLASS_BACKGROUND, 0), weakDoConnect);
+                UTMLog(@"SPICE port not in use yet, retries left: %d", tries);
+                dispatch_after(dispatch_time(DISPATCH_TIME_NOW, kRetryWait), dispatch_get_global_queue(QOS_CLASS_BACKGROUND, 0), weakDoConnect);
+                return;
             }
-        } else { // base case, no more tries
-            NSError *err = [NSError errorWithDomain:kUTMErrorDomain code:-1 userInfo:@{NSLocalizedDescriptionKey: NSLocalizedString(@"Failed to connect to SPICE server.", "UTMSpiceIO")}];
-            block(NO, err);
         }
+        // if we get here, error is unrecoverable
+        block(NO, NSLocalizedString(@"Failed to connect to SPICE server.", "UTMSpiceIO"));
+        weakSelf.connectionCallback = nil;
+        weakSelf.doConnect = nil;
     };
     weakDoConnect = self.doConnect;
     dispatch_async(dispatch_get_global_queue(QOS_CLASS_BACKGROUND, 0), self.doConnect);
@@ -167,8 +175,7 @@ typedef void (^connectionCallback_t)(BOOL success, NSError * _Nullable err);
 - (void)spiceError:(CSConnection *)connection err:(NSString *)msg {
     NSAssert(connection == self.spiceConnection, @"Unknown connection");
     if (self.connectionCallback) {
-        NSError *err = [NSError errorWithDomain:kUTMErrorDomain code:-1 userInfo:@{NSLocalizedDescriptionKey: msg}];
-        self.connectionCallback(NO, err);
+        self.connectionCallback(NO, msg);
         self.connectionCallback = nil;
     }
 }

Managers/UTMTerminalIO.m

@@ -36,7 +36,7 @@
     return [_terminal connectWithError: err];
 }
 
-- (void)connectWithCompletion: (void(^)(BOOL, NSError*)) block {
+- (void)connectWithCompletion: (void(^)(BOOL, NSString * _Nullable msg)) block {
     // there's no connection to be made, so just return YES
     block(YES, nil);
 }

Managers/UTMVirtualMachine.m

@@ -33,7 +33,7 @@
 #import "UTMPortAllocator.h"
 #import "qapi-events.h"
 
-const int kQMPMaxConnectionTries = 10; // qemu needs to start spice server first
+const int kQMPMaxConnectionTries = 30; // qemu needs to start spice server first
 const int64_t kStopTimeout = (int64_t)30*NSEC_PER_SEC;
 
 NSString *const kUTMErrorDomain = @"com.osy86.utm";
@@ -291,9 +291,9 @@ error:
         }
         dispatch_semaphore_signal(self->_qemu_exit_sema);
     }];
-    [self->_ioService connectWithCompletion:^(BOOL success, NSError * _Nullable error) {
+    [self->_ioService connectWithCompletion:^(BOOL success, NSString * _Nullable msg) {
         if (!success) {
-            [self errorTriggered:NSLocalizedString(@"Failed to connect to display server.", @"UTMVirtualMachine")];
+            [self errorTriggered:msg];
         } else {
             [self changeState:kVMStarted];
             [self restoreViewState];

QEMUHelper/QEMUHelper.m

@@ -20,8 +20,6 @@
 @interface QEMUHelper ()
 
 @property NSMutableArray<NSURL *> *urls;
-@property NSMutableDictionary<NSString *, NSTask *> *processes;
-@property NSMutableDictionary<NSString *, void(^)(BOOL, NSString *)> *exitHandlers;
 
 @end
 
@@ -30,7 +28,6 @@
 - (instancetype)init {
     if (self = [super init]) {
         self.urls = [NSMutableArray array];
-        self.processes = [NSMutableDictionary dictionary];
     }
     return self;
 }
@@ -73,11 +70,11 @@
     completion(YES, bookmark, url.path);
 }
 
-- (void)startQemu:(NSString *)binName libraryBookmark:(NSData *)libBookmark  argv:(NSArray<NSString *> *)argv onStarted:(void(^)(NSString * _Nullable))onStarted {
+- (void)startQemu:(NSString *)binName libraryBookmark:(NSData *)libBookmark argv:(NSArray<NSString *> *)argv onExit:(void(^)(BOOL,NSString *))onExit {
     NSURL *qemuURL = [[NSBundle mainBundle] URLForAuxiliaryExecutable:binName];
     if (!qemuURL || ![[NSFileManager defaultManager] fileExistsAtPath:qemuURL.path]) {
         NSLog(@"Cannot find executable for %@", binName);
-        onStarted(nil);
+        onExit(NO, NSLocalizedString(@"Cannot find QEMU executable.", @"QEMUHelper"));
         return;
     }
     
@@ -89,74 +86,22 @@
                                                      error:&err];
     if (!libraryPath || ![[NSFileManager defaultManager] fileExistsAtPath:libraryPath.path]) {
         NSLog(@"Cannot resolve library path: %@", err);
-        onStarted(nil);
+        onExit(NO, NSLocalizedString(@"Cannot find QEMU support libraries.", @"QEMUHelper"));
         return;
     }
     
     NSTask *task = [NSTask new];
-    NSString *identifier = [self storeProcess:task];
     task.executableURL = qemuURL;
     task.arguments = argv;
     task.environment = @{@"DYLD_LIBRARY_PATH": libraryPath.path};
     task.qualityOfService = NSQualityOfServiceUserInitiated;
     task.terminationHandler = ^(NSTask *task) {
-        [self exitProcessForIdentifier:identifier];
+        BOOL normalExit = task.terminationReason == NSTaskTerminationReasonExit && task.terminationStatus == 0;
+        onExit(normalExit, nil); // TODO: get last error line
     };
     if (![task launchAndReturnError:&err]) {
         NSLog(@"Error starting QEMU: %@", err);
-        [self exitProcessForIdentifier:identifier];
-        onStarted(nil);
-    } else {
-        onStarted(identifier);
-    }
-}
-
-- (void)registerExitHandlerForIdentifier:(NSString *)identifier handler:(void(^)(BOOL,NSString *))handler {
-    if (![self storeExitHandlerForIdentifier:identifier handler:handler]) {
-        handler(NO, NSLocalizedString(@"QEMU already exited.", @"QEMUHelper"));
-    }
-}
-
-#pragma mark - Helpers
-
-- (NSString *)storeProcess:(NSTask *)process {
-    @synchronized (self.processes) {
-        NSString *identifier = [NSUUID UUID].UUIDString;
-        self.processes[identifier] = process;
-        return identifier;
-    }
-}
-
-- (BOOL)storeExitHandlerForIdentifier:(NSString *)identifier handler:(void(^)(BOOL,NSString *))handler {
-    @synchronized (self.processes) {
-        if (self.processes[identifier]) {
-            self.exitHandlers[identifier] = handler;
-            return YES;
-        } else {
-            return NO;
-        }
-    }
-}
-
-- (void)exitProcessForIdentifier:(NSString *)identifier {
-    NSTask *task;
-    void (^exitHandler)(BOOL, NSString *);
-    BOOL normalExit = NO;
-    @synchronized (self.processes) {
-        task = self.processes[identifier];
-        [self.processes removeObjectForKey:identifier];
-        exitHandler = self.exitHandlers[identifier];
-        [self.exitHandlers removeObjectForKey:identifier];
-    }
-    if (task) {
-        if (task.running) {
-            [task terminate];
-        } else {
-            normalExit = task.terminationReason == NSTaskTerminationReasonExit && task.terminationStatus == 0;
-        }
-    }
-    if (exitHandler) {
-        exitHandler(normalExit, nil); // TODO: get last error line
+        onExit(NO, NSLocalizedString(@"Error starting QEMU.", @"QEMUHelper"));
     }
 }
 

QEMUHelper/QEMUHelperProtocol.h

@@ -22,8 +22,7 @@ NS_ASSUME_NONNULL_BEGIN
 @protocol QEMUHelperProtocol
 
 - (void)accessDataWithBookmark:(NSData *)bookmark securityScoped:(BOOL)securityScoped completion:(void(^)(BOOL, NSData * _Nullable, NSString * _Nullable))completion;
-- (void)startQemu:(NSString *)binName libraryBookmark:(NSData *)libBookmark argv:(NSArray<NSString *> *)argv onStarted:(void(^)(NSString * _Nullable))onStarted;
-- (void)registerExitHandlerForIdentifier:(NSString *)identifier handler:(void(^)(BOOL,NSString *))handler;
+- (void)startQemu:(NSString *)binName libraryBookmark:(NSData *)libBookmark argv:(NSArray<NSString *> *)argv onExit:(void(^)(BOOL,NSString *))onExit;
     
 @end