-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
macOS desktop app doesn't restart after updating #12433
Comments
What version did you update from, in case you recall? |
A recent version, I think one that was released a few days ago? |
Can confirm this issue on Catalina with the latest stable Riot version. Rolling back on Catalina does not help, then the old riot still does not show up. Only notifications work, the main view does not open. Update: We upgraded another Catalina Mac, worked on that laptop, so I'm not sure what the problem is. |
Squirrel/Squirrel.Mac#204 looks like the closest approximation of an upstream bug to track this |
I see this problem on macOS 10.14.4, don't think it's related to Catalina. |
I'm really wondering why we have this problem but Slack and VSCode, both of which afaik run on Electron, don't. I'm sure the fix is somewhere buried in https://github.com/microsoft/vscode ... https://github.com/microsoft/vscode/blob/master/build/lib/electron.ts#L116 maybe? |
I would guess at least VS Code uses a custom updater pipeline instead of Squirrel...? |
Main window is gone, "Show All Windows" shows nothing. macOS Catalina $ /Applications/Riot.app/Contents/MacOS/Riot
Starting auto update with base URL: https://packages.riot.im/desktop/update/
(electron) The default value of app.allowRendererProcessReuse is deprecated, it is currently "false". It will change to be "true" in Electron 9. For more information please check https://github.com/electron/electron/issues/18397
GVA encoder info: AMD performance mode : 2
GVA encoder info: deleteSCDMetalContext : texture cache hits: 0, misses: 0 update 1: Main window shows when specifying empty dir as profile-dir. $ /Applications/Riot.app/Contents/MacOS/Riot --profile-dir $PWD
Starting auto update with base URL: https://packages.riot.im/desktop/update/
(electron) The default value of app.allowRendererProcessReuse is deprecated, it is currently "false". It will change to be "true" in Electron 9. For more information please check https://github.com/electron/electron/issues/18397
GVA encoder info: AMD performance mode : 2
GVA encoder info: deleteSCDMetalContext : texture cache hits: 0, misses: 0 This resolved the issue for me: rm ~/Library/Application\ Support/Riot/window-state.json |
This now works for me since a few days/weeks ago, without any modifications/hacks. |
This failed for me this morning |
@kegsay @benparsons Are you using nightly or release? |
Nightly |
FYI of two prompted restarts today, the first failed, the second succeeded. |
Okay, sounds like something's still racing / wrong here. |
@jryans are there logs or anything else I can offer as help? I (obsessively 😅) pay attention to whether it succeeds or not but don't see any pattern... |
Release. |
@benparsons Not sure yet, but we're discussing as a team, we'll try to work out a next step here. |
Didn't reopen after my last update :( |
Related #12402 #6252 (based on #12433 (comment)) |
empirically this seems to have inexplicably started working at last, in the last week or so, presumably due to something in electron or squirrel having fixed itself. folks: please yell if you see it recur; otherwise let's close this in a bit. |
...or i could just press the wrong button and close it now, i guess. |
Definitely feels like a race condition due to how it sometimes does and sometimes doesn't. I have the same behaviour. |
1.7.27 update on OSX didn't automatically relaunch after completion |
I've been encountering this too. I don't know how ShipIt works, but I found n.b. I think the A successful update:
A failed update:
|
this affects almost every update for me; the only time it manages to successfully update is if the upgrade is <24h since the last upgrade. i assume it also affects updates for stable as well as nightly. surely this should be higher priority? |
I just took a look into a typical failed update:
Interesting things to note:
In other words, However, the fact that launchAfterInstallation is false there seems suspicious. Next step: compare the state file with the next successful install+launch cycle, and see how it differs. Alternative theory: if ShipIt is invoked while the laptop is off, perhaps the main thread times out somehow with some timeout while waiting for the user to hit 'update'. |
Another observation - Electron is applying a bunch of patches to the main squirrel.mac tree, especially stuff like https://github.com/electron/electron/blob/main/patches/squirrel.mac/feat_add_new_squirrel_mac_bundle_installation_method_behind_flag.patch, which explains why the logging doesn't match the squirrel.mac source tree, and might contribute to the problems here. |
Theory: electron's AutoUpdater::QuitAndInstall calls relaunchToInstallUpdate on Squirrel to install the app. This fairly unambiguously should set However, empirically, the state file isn't being written at the point that QuitAndInstall is called - we're seeing an earlier copy of the state file on disk. Looking at: - (RACSignal *)relaunchToInstallUpdate {
return [[[[[[[[SQRLShipItRequest
readUsingURL:self.shipItStateURL]
map:^(SQRLShipItRequest *request) {
return [[SQRLShipItRequest alloc] initWithUpdateBundleURL:request.updateBundleURL targetBundleURL:request.targetBundleURL bundleIdentifier:request.bundleIdentifier launchAfterInstallation:YES useUpdateBundleName:request.useUpdateBundleName];
}]
flattenMap:^(SQRLShipItRequest *request) {
return [[request
writeUsingURL:self.shipItStateURL]
sqrl_addTransactionWithName:NSLocalizedString(@"Preparing to relaunch", nil) description:NSLocalizedString(@"%@ is preparing to relaunch to install an update. Interrupting the process could corrupt the application.", nil), NSRunningApplication.currentApplication.bundleIdentifier];
}]
deliverOn:RACScheduler.mainThreadScheduler]
doCompleted:^{
[NSApp terminate:self];
}]
// Never allow `completed` to escape this signal chain (in case
// -terminate: is asynchronous or something crazy).
concat:[RACSignal never]]
replay]
setNameWithFormat:@"%@ -relaunchToInstallUpdate", self];
} I wonder whether the |
Another thought: https://github.com/vector-im/element-desktop/blob/develop/src/updater.ts has pollforupdates being triggered by both electron’s callback and from our timer. i wonder if we have a race between the two? |
I captured a successful upgrade just now. (The app hadn't spotted an upgrade overnight, for some reason; i had to manually click the check for upgrade button on opening it). After having downloaded the update, the state file was: {
"launchAfterInstallation": false,
"updateBundleURL": "file:///Users/matthew/Library/Caches/im.riot.nightly.ShipIt/update.J8xom8S/Element%20Nightly.app/",
"useUpdateBundleName": true,
"bundleIdentifier": "im.riot.nightly",
"targetBundleURL": "file:///Applications/Element%20Nightly.app/"
} but after installing the update successfully & reloaunching, the state file had been correctly updated to: {
"launchAfterInstallation": true,
"updateBundleURL": "file:///Users/matthew/Library/Caches/im.riot.nightly.ShipIt/update.J8xom8S/Element%20Nightly.app/",
"useUpdateBundleName": true,
"bundleIdentifier": "im.riot.nightly",
"targetBundleURL": "file:///Applications/Element%20Nightly.app/"
} The logs from stderr.log were:
I wonder if there's a problem that the install request happens when it does the check and does the download, rather than when you actually hit the install button - which could be a long time apart? Finally, for completeness, i left the Console running filtered on ShipIt, in case it captured anything else (and for a comparison with a bad run). Output was:
|
This is a punt at fixing element-hq/element-web#12433, on the assumption that multiple update checks might collide with a download which are already queued to install. It also avoids repeatedly re-downloading the same update on every check, as per the Note: on https://github.com/electron/electron/blob/main/docs/api/auto-updater.md#autoupdatercheckforupdates However, it means that you may have to upgrade twice if you wait more than 24h to install a new build - and if you cancel an upgrade prompt, you'll have to either restart the app or explicitly check for a new version to get upgrades working again. However, this is less annoying than having the app fail to relaunch after upgrading.
element-hq/element-desktop#386 is a stab at fixing this, by stopping |
Closing as presumed fixed, if anyone sees this recur after 1.11.1 please shout |
I haven't had a failure since the fix landed, so yes, it looks speculatively fixed. Will reopen if needed. For completeness, here's a ShipIt log from console of today's successful update:
|
FWIW, my install has also updated reliably and consistently every day since. Thank you! |
@ara4n We're having the same issue, and the same work around fixes it. This is not great though as we have some users that don't auto update our app often, and so can end up many versions behind. When we then eventually force updates those users end up on the version they downloaded weeks or months ago, rather than on the most recent update. Any chance you found a different solution to this that allowed a second version to be downloaded? |
For anyone who finds this in the future. I dug in more, and found that as long as the last call to
|
@seanmacisaac thanks for your feedback with the workaround. Do you know if there's any upstream issue? Have adopted a workaround based on your snippet in element-hq/element-desktop#629 |
I don't, and I haven't spent the time needed to track down whether the problem is with Electron or its patching of Squirrel.Mac or Squirrel.Mac itself, or some lower layer than that. I did see the behavior noted here Squirrel/Squirrel.Mac#204 (comment), but I'm not sure that is really a Squirrel.Mac issue vs an Electron issue. |
OS: Mac Catalina
Basically what it says in the title. Yes, I've waited a while.
The text was updated successfully, but these errors were encountered: