electron · codebytere · Nov 16, 2022 · Nov 14, 2022
@@ -3,3 +3,4 @@ fix_ensure_that_self_is_retained_until_the_racsignal_is_complete.patch
 fix_use_kseccschecknestedcode_kseccsstrictvalidate_in_the_sec.patch
 feat_add_new_squirrel_mac_bundle_installation_method_behind_flag.patch
 refactor_use_posix_spawn_instead_of_nstask_so_we_can_disclaim_the.patch
+fix_abort_installation_attempt_at_the_final_mile_if_the_app_is.patch
@@ -0,0 +1,75 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Samuel Attard <sattard@salesforce.com>
+Date: Tue, 25 Oct 2022 13:09:55 -0700
+Subject: fix: abort installation attempt at the final mile if the app is
+ running
+
+There is a race condition between ShipIt launching and it performing an atomic rename to "install" the app bundle.  This fixes the race by checking the list of running apps immediately before performing the rename.
+
+diff --git a/Squirrel/SQRLInstaller.h b/Squirrel/SQRLInstaller.h
+index 2de1c384aae200f41de429cc35313e4c2ba9d0de..35a0c99129478f09526667d73c9544c3bfe14706 100644
+--- a/Squirrel/SQRLInstaller.h
++++ b/Squirrel/SQRLInstaller.h
+@@ -37,6 +37,9 @@ extern const NSInteger SQRLInstallerErrorMovingAcrossVolumes;
+ // There was an error changing the file permissions of the update.
+ extern const NSInteger SQRLInstallerErrorChangingPermissions;
+
++// There was a running instance of the app just prior to the update attempt
++extern const NSInteger SQRLInstallerErrorAppStillRunning;
++
+ @class RACCommand;
+
+ // Performs the installation of an update, saving its intermediate state to user
+diff --git a/Squirrel/SQRLInstaller.m b/Squirrel/SQRLInstaller.m
+index c1f328fa8c3689218ef260347cb8f9d30b789efe..f502df2f88424ea902a061adfeb30358daf212e4 100644
+--- a/Squirrel/SQRLInstaller.m
++++ b/Squirrel/SQRLInstaller.m
+@@ -36,6 +36,7 @@
+ const NSInteger SQRLInstallerErrorInvalidState = -6;
+ const NSInteger SQRLInstallerErrorMovingAcrossVolumes = -7;
+ const NSInteger SQRLInstallerErrorChangingPermissions = -8;
++const NSInteger SQRLInstallerErrorAppStillRunning = -9;
+
+ NSString * const SQRLShipItInstallationAttemptsKey = @"SQRLShipItInstallationAttempts";
+ NSString * const SQRLInstallerOwnedBundleKey = @"SQRLInstallerOwnedBundle";
+@@ -286,6 +287,19 @@ - (RACSignal *)installRequest:(SQRLShipItRequest *)request {
+ 			return [[[[self
+ 				renameIfNeeded:request updateBundleURL:updateBundleURL]
+ 				flattenMap:^(SQRLShipItRequest *request) {
++					// Final validation that the application is not running again;
++					NSArray *apps = [[NSRunningApplication runningApplicationsWithBundleIdentifier:request.bundleIdentifier] filteredArrayUsingPredicate:[NSPredicate predicateWithBlock:^BOOL(NSRunningApplication *app, NSDictionary *bindings) {
++						return [[[app bundleURL] URLByStandardizingPath] isEqual:request.targetBundleURL];
++					}]];
++					if ([apps count] != 0) {
++						NSLog(@"Aborting update attempt because there are %lu running instances of the target app", [apps count]);
++						NSDictionary *errorInfo = @{
++							NSLocalizedDescriptionKey: NSLocalizedString(@"App Still Running Error", nil),
++							NSLocalizedRecoverySuggestionErrorKey: NSLocalizedString(@"All instances of the target application should be quit during the update process", nil),
++						};
++						return [RACSignal error:[NSError errorWithDomain:SQRLInstallerErrorDomain code:SQRLInstallerErrorAppStillRunning userInfo:errorInfo]];
++					}
++
+ 					return [[self acquireTargetBundleURLForRequest:request] concat:[RACSignal return:request]];
+ 				}]
+ 				flattenMap:^(SQRLShipItRequest *request) {
+diff --git a/Squirrel/ShipIt-main.m b/Squirrel/ShipIt-main.m
+index 2c515ffdd67052a08ee8155c0e46b57e9721a0e5..5f3e29642012d04fc506b730a4e87fba861df250 100644
+--- a/Squirrel/ShipIt-main.m
++++ b/Squirrel/ShipIt-main.m
+@@ -201,8 +201,14 @@ static void installRequest(RACSignal *readRequestSignal, NSString *applicationId
+ 			return action;
+ 		}]
+ 		subscribeError:^(NSError *error) {
+-			NSLog(@"Installation error: %@", error);
+-			exit(EXIT_FAILURE);
++			if ([[error domain] isEqual:SQRLInstallerErrorDomain] && [error code] == SQRLInstallerErrorAppStillRunning) {
++				NSLog(@"Installation cancelled: %@", error);
++				clearInstallationAttempts(applicationIdentifier);
++				exit(EXIT_SUCCESS);
++			} else {
++				NSLog(@"Installation error: %@", error);
++				exit(EXIT_FAILURE);
++			}
+ 		} completed:^{
+ 			exit(EXIT_SUCCESS);
+ 		}];
diff --git a/spec/api-autoupdater-darwin-spec.ts b/spec/api-autoupdater-darwin-spec.ts
@@ -5,6 +5,7 @@ import * as express from 'express';
 import * as fs from 'fs-extra';
 import * as os from 'os';
 import * as path from 'path';
+import * as psList from 'ps-list';
 import { AddressInfo } from 'net';
 import { ifdescribe, ifit } from './spec-helpers';
 import * as uuid from 'uuid';
@@ -95,6 +96,16 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
     return spawn(path.resolve(appPath, 'Contents/MacOS/Electron'), args);
   };
 
+  const spawnAppWithHandle = (appPath: string, args: string[] = []) => {
+    return cp.spawn(path.resolve(appPath, 'Contents/MacOS/Electron'), args);
+  };
+
+  const getRunningShipIts = async (appPath: string) => {
+    const processes = await psList();
+    const activeShipIts = processes.filter(p => p.cmd?.includes('Squirrel.framework/Resources/ShipIt com.github.Electron.ShipIt') && p.cmd!.startsWith(appPath));
+    return activeShipIts;
+  };
+
   const withTempDirectory = async (fn: (dir: string) => Promise<void>, autoCleanUp = true) => {
     const dir = await fs.mkdtemp(path.resolve(os.tmpdir(), 'electron-update-spec-'));
     try {
@@ -323,6 +334,71 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
       });
     });
 
+    it('should abort the update if the application is still running when ShipIt kicks off', async () => {
+      await withUpdatableApp({
+        nextVersion: '2.0.0',
+        startFixture: 'update',
+        endFixture: 'update'
+      }, async (appPath, updateZipPath) => {
+        server.get('/update-file', (req, res) => {
+          res.download(updateZipPath);
+        });
+        server.get('/update-check', (req, res) => {
+          res.json({
+            url: `http://localhost:${port}/update-file`,
+            name: 'My Release Name',
+            notes: 'Theses are some release notes innit',
+            pub_date: (new Date()).toString()
+          });
+        });
+
+        enum FlipFlop {
+          INITIAL,
+          FLIPPED,
+          FLOPPED,
+        }
+
+        const shipItFlipFlopPromise = new Promise<void>((resolve) => {
+          let state = FlipFlop.INITIAL;
+          const checker = setInterval(async () => {
+            const running = await getRunningShipIts(appPath);
+            switch (state) {
+              case FlipFlop.INITIAL: {
+                if (running.length) state = FlipFlop.FLIPPED;
+                break;
+              }
+              case FlipFlop.FLIPPED: {
+                if (!running.length) state = FlipFlop.FLOPPED;
+                break;
+              }
+            }
+            if (state === FlipFlop.FLOPPED) {
+              clearInterval(checker);
+              resolve();
+            }
+          }, 500);
+        });
+
+        const launchResult = await launchApp(appPath, [`http://localhost:${port}/update-check`]);
+        const retainerHandle = spawnAppWithHandle(appPath, ['remain-open']);
+        logOnError(launchResult, () => {
+          expect(launchResult).to.have.property('code', 0);
+          expect(launchResult.out).to.include('Update Downloaded');
+          expect(requests).to.have.lengthOf(2);
+          expect(requests[0]).to.have.property('url', '/update-check');
+          expect(requests[1]).to.have.property('url', '/update-file');
+          expect(requests[0].header('user-agent')).to.include('Electron/');
+          expect(requests[1].header('user-agent')).to.include('Electron/');
+        });
+
+        await shipItFlipFlopPromise;
+        expect(requests).to.have.lengthOf(2, 'should not have relaunched the updated app');
+        expect(JSON.parse(await fs.readFile(path.resolve(appPath, 'Contents/Resources/app/package.json'), 'utf8')).version).to.equal('1.0.0', 'should still be the old version on disk');
+
+        retainerHandle.kill('SIGINT');
+      });
+    });
+
     describe('with SquirrelMacEnableDirectContentsWrite enabled', () => {
       let previousValue: any;
 

diff --git a/spec/fixtures/auto-update/update/index.js b/spec/fixtures/auto-update/update/index.js
@@ -15,28 +15,34 @@ autoUpdater.on('error', (err) => {
 
 const urlPath = path.resolve(__dirname, '../../../../url.txt');
 let feedUrl = process.argv[1];
-if (!feedUrl || !feedUrl.startsWith('http')) {
-  feedUrl = `${fs.readFileSync(urlPath, 'utf8')}/${app.getVersion()}`;
+
+if (feedUrl === 'remain-open') {
+  // Hold the event loop
+  setInterval(() => {});
 } else {
-  fs.writeFileSync(urlPath, `${feedUrl}/updated`);
+  if (!feedUrl || !feedUrl.startsWith('http')) {
+    feedUrl = `${fs.readFileSync(urlPath, 'utf8')}/${app.getVersion()}`;
+  } else {
+    fs.writeFileSync(urlPath, `${feedUrl}/updated`);
+  }
+
+  autoUpdater.setFeedURL({
+    url: feedUrl
+  });
+
+  autoUpdater.checkForUpdates();
+
+  autoUpdater.on('update-available', () => {
+    console.log('Update Available');
+  });
+
+  autoUpdater.on('update-downloaded', () => {
+    console.log('Update Downloaded');
+    autoUpdater.quitAndInstall();
+  });
+
+  autoUpdater.on('update-not-available', () => {
+    console.error('No update available');
+    process.exit(1);
+  });
 }
-
-autoUpdater.setFeedURL({
-  url: feedUrl
-});
-
-autoUpdater.checkForUpdates();
-
-autoUpdater.on('update-available', () => {
-  console.log('Update Available');
-});
-
-autoUpdater.on('update-downloaded', () => {
-  console.log('Update Downloaded');
-  autoUpdater.quitAndInstall();
-});
-
-autoUpdater.on('update-not-available', () => {
-  console.error('No update available');
-  process.exit(1);
-});
diff --git a/spec/package.json b/spec/package.json
@@ -24,6 +24,7 @@
     "mocha-junit-reporter": "^1.18.0",
     "mocha-multi-reporters": "^1.1.7",
     "pdfjs-dist": "^2.2.228",
+    "ps-list": "^7.0.0",
     "q": "^1.5.1",
     "send": "^0.16.2",
     "sinon": "^9.0.1",

diff --git a/spec/yarn.lock b/spec/yarn.lock
@@ -73,10 +73,9 @@
   dependencies:
     "@types/node" "*"
 
-abstract-socket@^2.0.0:
-  version "2.1.1"
-  resolved "https://registry.yarnpkg.com/abstract-socket/-/abstract-socket-2.1.1.tgz#243a7e6e6ff65bb9eab16a22fa90699b91e528f7"
-  integrity sha512-YZJizsvS1aBua5Gd01woe4zuyYBGgSMeqDOB6/ChwdTI904KP6QGtJswXl4hcqWxbz86hQBe++HWV0hF1aGUtA==
+"abstract-socket@github:saghul/node-abstractsocket#35b1b1491fabc04899bde5be3428abf5cf9cd528":
+  version "2.1.0"
+  resolved "https://codeload.github.com/saghul/node-abstractsocket/tar.gz/35b1b1491fabc04899bde5be3428abf5cf9cd528"
   dependencies:
     bindings "^1.2.1"
     nan "^2.12.1"
@@ -385,7 +384,7 @@ data-uri-to-buffer@0.0.3:
     safe-buffer "^5.1.1"
     xml2js "^0.4.17"
   optionalDependencies:
-    abstract-socket "^2.0.0"
+    abstract-socket "github:saghul/node-abstractsocket#35b1b1491fabc04899bde5be3428abf5cf9cd528"
 
 debug@2.6.9, debug@^2.2.0:
   version "2.6.9"
@@ -1283,6 +1282,11 @@ pngjs@^3.3.3:
   resolved "https://registry.yarnpkg.com/pngjs/-/pngjs-3.4.0.tgz#99ca7d725965fb655814eaf65f38f12bbdbf555f"
   integrity sha512-NCrCHhWmnQklfH4MtJMRjZ2a8c80qXeMlQMv2uVp9ISJMTt562SbGd6n2oq0PaPgKm7Z6pL9E2UlLIhC+SHL3w==
 
+ps-list@^7.0.0:
+  version "7.2.0"
+  resolved "https://registry.yarnpkg.com/ps-list/-/ps-list-7.2.0.tgz#3d110e1de8249a4b178c9b1cf2a215d1e4e42fc0"
+  integrity sha512-v4Bl6I3f2kJfr5o80ShABNHAokIgY+wFDTQfE+X3zWYgSGQOCBeYptLZUpoOALBqO5EawmDN/tjTldJesd0ujQ==
+
 psl@^1.1.24:
   version "1.9.0"
   resolved "https://registry.yarnpkg.com/psl/-/psl-1.9.0.tgz#d0df2a137f00794565fcaf3b2c00cd09f8d5a5a7"