Wait for a threshold amount of time before replying with status info data

Autoupdate/AppInstaller.m

@@ -34,7 +34,6 @@
 
 #define FIRST_UPDATER_MESSAGE_TIMEOUT 18ull
 #define RETRIEVE_PROCESS_IDENTIFIER_TIMEOUT 8ull
-#define UPDATER_APPCAST_ITEM_REGISTRATION_TIMEOUT 8ull
 
 /**
  * Show display progress UI after a delay from starting the final part of the installation.
@@ -422,40 +421,11 @@ - (void)handleMessageWithIdentifier:(int32_t)identifier data:(NSData *)data
     } else if (identifier == SPUSentUpdateAppcastItemData) {
         SUAppcastItem *updateItem = (SUAppcastItem *)SPUUnarchiveRootObjectSecurely(data, [SUAppcastItem class]);
         if (updateItem != nil) {
-            BOOL canInstallSilently = [self.installer canInstallSilently];
-            SPUInstallationInfo *installationInfo = [[SPUInstallationInfo alloc] initWithAppcastItem:updateItem canSilentlyInstall:canInstallSilently];
+            SPUInstallationInfo *installationInfo = [[SPUInstallationInfo alloc] initWithAppcastItem:updateItem canSilentlyInstall:[self.installer canInstallSilently]];
 
             NSData *archivedData = SPUArchiveRootObjectSecurely(installationInfo);
             if (archivedData != nil) {
-                __block BOOL notifiedRegistrationCompleted = NO;
-
-                [self.agentConnection.agent registerInstallationInfoData:archivedData completionHandler:^{
-                    dispatch_async(dispatch_get_main_queue(), ^{
-                        if (!notifiedRegistrationCompleted) {
-                            notifiedRegistrationCompleted = YES;
-
-                            uint8_t targetTerminated = (uint8_t)self.terminationListener.terminated;
-                            uint8_t sendInformation[] = {targetTerminated, (uint8_t)canInstallSilently};
-
-                            [self.communicator handleMessageWithIdentifier:SPUInstallerRegisteredAppcastItem data:[NSData dataWithBytes:sendInformation length:sizeof(sendInformation)]];
-                        }
-                    });
-                }];
-
-                // Not receiving that we've registered the appcast item in a timely manner is not a fatal issue
-                // If this operation takes too long, just let assume the item will be registered
-                dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(UPDATER_APPCAST_ITEM_REGISTRATION_TIMEOUT * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
-                    if (!notifiedRegistrationCompleted) {
-                        notifiedRegistrationCompleted = YES;
-
-                        SULog(SULogLevelError, @"Warning: did not receive appcast item registration from Updater in timely manner. Proceeding..");
-
-                        uint8_t targetTerminated = (uint8_t)self.terminationListener.terminated;
-                        uint8_t sendInformation[] = {targetTerminated, (uint8_t)canInstallSilently};
-
-                        [self.communicator handleMessageWithIdentifier:SPUInstallerRegisteredAppcastItem data:[NSData dataWithBytes:sendInformation length:sizeof(sendInformation)]];
-                    }
-                });
+                [self.agentConnection.agent registerInstallationInfoData:archivedData];
             }
         }
     } else if (identifier == SPUResumeInstallationToStage2 && data.length == sizeof(uint8_t) * 2) {
@@ -512,10 +482,18 @@ - (void)startInstallation
             return;
         }
 
+        uint8_t canPerformSilentInstall = (uint8_t)[installer canInstallSilently];
+
         dispatch_async(dispatch_get_main_queue(), ^{
             self.installer = installer;
 
-            [self.communicator handleMessageWithIdentifier:SPUInstallationFinishedStage1 data:[NSData data]];
+            uint8_t targetTerminated = (uint8_t)self.terminationListener.terminated;
+
+            uint8_t sendInformation[] = {canPerformSilentInstall, targetTerminated};
+
+            NSData *sendData = [NSData dataWithBytes:sendInformation length:sizeof(sendInformation)];
+
+            [self.communicator handleMessageWithIdentifier:SPUInstallationFinishedStage1 data:sendData];
 
             self.performedStage1Installation = YES;
 

Autoupdate/SPUMessageTypes.h

@@ -23,8 +23,7 @@ typedef NS_ENUM(int32_t, SPUInstallerMessageType)
     SPUInstallationFinishedStage2 = 7,
     SPUInstallationFinishedStage3 = 8,
     SPUUpdaterAlivePing = 9,
-    SPUInstallerError = 10,
-    SPUInstallerRegisteredAppcastItem = 11,
+    SPUInstallerError = 10
 };
 
 typedef NS_ENUM(int32_t, SPUUpdaterMessageType)

Autoupdate/SPUMessageTypes.m

@@ -23,7 +23,7 @@
 
 BOOL SPUInstallerMessageTypeIsLegal(SPUInstallerMessageType oldMessageType, SPUInstallerMessageType newMessageType)
 {
-    BOOL legal = NO;
+    BOOL legal;
     switch (newMessageType) {
         case SPUInstallerNotStarted:
             legal = (oldMessageType == SPUInstallerNotStarted);
@@ -45,7 +45,7 @@ BOOL SPUInstallerMessageTypeIsLegal(SPUInstallerMessageType oldMessageType, SPUI
             legal = (oldMessageType == SPUInstallationStartedStage1);
             break;
         case SPUInstallationFinishedStage2:
-            legal = (oldMessageType == SPUInstallationFinishedStage1 || oldMessageType == SPUInstallerRegisteredAppcastItem);
+            legal = (oldMessageType == SPUInstallationFinishedStage1);
             break;
         case SPUInstallationFinishedStage3:
             legal = (oldMessageType == SPUInstallationFinishedStage2);
@@ -56,9 +56,6 @@ BOOL SPUInstallerMessageTypeIsLegal(SPUInstallerMessageType oldMessageType, SPUI
             // So just always allow these type of messages
             legal = YES;
             break;
-        case SPUInstallerRegisteredAppcastItem:
-            legal = (oldMessageType == SPUInstallationFinishedStage1);
-            break;
     }
     return legal;
 }

Autoupdate/StatusInfo.m

@@ -12,15 +12,18 @@
 
 #include "AppKitPrevention.h"
 
-@interface StatusInfo () <NSXPCListenerDelegate>
-
-@property (nonatomic) NSXPCListener *xpcListener;
+#define REPLY_STATUS_INFO_TIMEOUT 2
 
+@interface StatusInfo () <NSXPCListenerDelegate>
 @end
 
 @implementation StatusInfo
+{
+    NSXPCListener *_xpcListener;
+    NSMutableDictionary *_pendingReplies;
+    NSUInteger _pendingReplyCounter;
+}
 
-@synthesize xpcListener = _xpcListener;
 @synthesize installationInfoData = _installationInfoData;
 
 - (instancetype)initWithHostBundleIdentifier:(NSString *)bundleIdentifier
@@ -29,19 +32,20 @@ - (instancetype)initWithHostBundleIdentifier:(NSString *)bundleIdentifier
     if (self != nil) {
         _xpcListener = [[NSXPCListener alloc] initWithMachServiceName:SPUStatusInfoServiceNameForBundleIdentifier(bundleIdentifier)];
         _xpcListener.delegate = self;
+        _pendingReplies = [NSMutableDictionary dictionary];
     }
     return self;
 }
 
 - (void)startListener
 {
-    [self.xpcListener resume];
+    [_xpcListener resume];
 }
 
 - (void)invalidate
 {
-    [self.xpcListener invalidate];
-    self.xpcListener = nil;
+    [_xpcListener invalidate];
+    _xpcListener = nil;
 }
 
 - (BOOL)listener:(NSXPCListener *)__unused listener shouldAcceptNewConnection:(NSXPCConnection *)newConnection
@@ -54,10 +58,41 @@ - (BOOL)listener:(NSXPCListener *)__unused listener shouldAcceptNewConnection:(N
     return YES;
 }
 
+- (void)setInstallationInfoData:(NSData *)installationInfoData
+{
+    _installationInfoData = installationInfoData;
+
+    // Respond to all of our pending replies
+    for (NSNumber *replyKey in _pendingReplies) {
+        void (^replyBlock)(NSData * _Nullable) = self->_pendingReplies[replyKey];
+        replyBlock(_installationInfoData);
+    }
+
+    [_pendingReplies removeAllObjects];
+}
+
 - (void)probeStatusInfoWithReply:(void (^)(NSData * _Nullable))reply
 {
     dispatch_async(dispatch_get_main_queue(), ^{
-        reply(self.installationInfoData);
+        if (self->_installationInfoData != nil) {
+            reply(self->_installationInfoData);
+        } else {
+            // If we don't have the installation info data currently, we may receive it in a very short window afterwards
+            // In this case wait a bit for the reply. If we receive the data it will be in -setInstallationInfoData:
+            NSUInteger currentReplyCounter = self->_pendingReplyCounter;
+            self->_pendingReplyCounter++;
+
+            NSNumber *currentReplyCounterKey = @(currentReplyCounter);
+            self->_pendingReplies[currentReplyCounterKey] = [reply copy];
+
+            dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(REPLY_STATUS_INFO_TIMEOUT * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
+                void (^replyBlock)(NSData * _Nullable) = self->_pendingReplies[currentReplyCounterKey];
+                if (replyBlock != nil) {
+                    replyBlock(self->_installationInfoData);
+                    [self->_pendingReplies removeObjectForKey:currentReplyCounterKey];
+                }
+            });
+        }
     });
 }
 

Documentation/Installation.md

@@ -84,9 +84,7 @@ Otherwise a `SPUInstallationFinishedStage1` message is sent back to the updater
 The installer then listens and waits for the target application to relaunch terminates. If it is already terminated, then it resumes to stage 2 and 3 of the installation immediately on the assumption that the installer does not have permission to show UI interaction to the user. Thus if the installer has to show user interaction here and hasn't received an OK from the updater (it won't if the target application is already terminated), the install will fail. If the target is already terminated, the installer will also assume that the target should not be relaunched after installation.
 
 ### Installation Waiting Period
-The updater receives `SPUInstallationFinishedStage1` message. The updater sends a message `SPUSentUpdateAppcastItemData` with the appcast data in case the updater may request for it later (due to installer resumability, discussed later).
-
-The updater waits to receive a `SPUInstallerRegisteredAppcastItem` message indicating the installer registered receiving the appcast item data (it waits for this registration to avoid a race of asking for it later and not being provided with info). The updater also reads if the target has already been terminated (implying that the installer will continue installing the update immediately), and if the installation will be done silently.
+The updater receives `SPUInstallationFinishedStage1` message. The updater sends a message `SPUSentUpdateAppcastItemData` with the appcast data in case the updater may request for it later (due to installer resumability, discussed later). It also reads if the target has already been terminated (implying that the installer will continue installing the update immediately), and if the installation will be done silently.
 
 For UI based update drivers, the updater tells the user driver to show that the application is ready to be relaunched - the user can continue to install & relaunch the app. The user driver is only alerted however if the installation isn't happening immediately (that is, if the target application to relaunch is still alive). The user driver can decide whether to a) install b) install & relaunch or c) delay installation. If installation is delayed, it can be resumed later, or if the target application terminates, the installer will try to continue installation if it is capable to without user interaction.
 

Sparkle/InstallerProgress/InstallerProgressAppController.m

@@ -284,7 +284,7 @@ - (void)registerApplicationBundlePath:(NSString *)applicationBundlePath reply:(v
     });
 }
 
-- (void)registerInstallationInfoData:(NSData *)installationInfoData completionHandler:(void (^)(void))completionHandler
+- (void)registerInstallationInfoData:(NSData *)installationInfoData
 {
     dispatch_async(dispatch_get_main_queue(), ^{
         if (self.statusInfo.installationInfoData == nil && installationInfoData != nil) {
@@ -297,8 +297,6 @@ - (void)registerInstallationInfoData:(NSData *)installationInfoData completionHa
                 SULog(SULogLevelError, @"Error: Failed to decode initial installation info from installer: %@", installationInfoData);
             }
         }
-
-        completionHandler();
     });
 }
 

Sparkle/InstallerProgress/SPUInstallerAgentProtocol.h

@@ -14,7 +14,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)registerApplicationBundlePath:(NSString *)applicationBundlePath reply:(void (^)(NSNumber * _Nullable processIdentifier))reply;
 
-- (void)registerInstallationInfoData:(NSData *)installationInfoData completionHandler:(void (^)(void))completionHandler;
+- (void)registerInstallationInfoData:(NSData *)installationInfoData;
 
 - (void)sendTerminationSignal;
 

Sparkle/SPUInstallerDriver.m

@@ -316,17 +316,15 @@ - (void)_handleMessageWithIdentifier:(int32_t)identifier data:(NSData *)data
         } else {
             SULog(SULogLevelError, @"Error: Archived data to send for appcast item is nil");
         }
-    } else if (identifier == SPUInstallerRegisteredAppcastItem) {
-        self.currentStage = identifier;
 
-        BOOL hasTargetTerminated = NO;
+        BOOL canInstallSilently = NO;
         if (data.length >= sizeof(uint8_t)) {
-            hasTargetTerminated = (BOOL)*(const uint8_t *)data.bytes;
+            canInstallSilently = (BOOL)*(const uint8_t *)data.bytes;
         }
 
-        BOOL canInstallSilently = NO;
+        BOOL hasTargetTerminated = NO;
         if (data.length >= sizeof(uint8_t) * 2) {
-            canInstallSilently = (BOOL)*((const uint8_t *)data.bytes + 1);
+            hasTargetTerminated = (BOOL)*((const uint8_t *)data.bytes + 1);
         }
 
         [self.delegate installerDidFinishPreparationAndWillInstallImmediately:hasTargetTerminated silently:canInstallSilently];