improved interoperability with app-updaters

[core-code]

this is not a bug report per-se but rather a request for an enhancement that would be in the best interest of users of apps using Sparkle.

Sparkle allows apps to implement auto-update functionality, which is great. however, some users prefer to have software that can keep all their apps up-to-date at once as this has various benefits. we are offering one of those products [1], but there are other solutions on the market, both open-source as well as commercial.

all in all, our product works great [2] and can update more than 5000 apps with the click of a button. however, there is a rare problem when our product tries to update an app, while the app also tries to update itself at the very same point-in-time using Sparkle. this can lead to failures on either end, and i guess in the absolute worst case it could even leave the user with a destroyed app. this is uncool.

this report is about preventing this problem in the interest of all users of apps with Sparkle that also use an app-updater (ours or any other).

note that we try to be /very/ careful when updating an app and we have a long array of protection-measures [e.g. we have a good look if any org.sparkle-project.* process is running inside the app-bundle] and sanity-checking but our logfiles still indicate that some updates fail because a concurrent update has been performed.

this mainly affects those apps that have configured Sparkle to perform updates in a transparent fashion. in this case our app will ask the app-to-be-updated to quit, in order update it - but this is exactly the time when your transparent update-at-quit code kicks in: perfect collision course. but it also affects those apps using Sparkle in their stock settings if the user has asked the app to update itself while asking our app to update it too, at the very same time. yes, users will do nonsense like that and there is no way to prevent it, so we should deal with it gracefully.

long story short we'd like to prose a solution to avoid any app-updater trying to update an app at the very same time as it also tries to update itself. i imagine a solution has two parts:
1.) a way for Sparkle to tell the outside world that it is going to update the host app. if this 'flag' is set then no outside app-updater should try to update this app
2.) a way for app-updaters to tell Sparkle not to update its host app because an update is already being performed by the app-updater
so, whoever comes first "wins" and should continue doing the update without having to fear that the other party will do it at the same time. basically we need a mutex/lock for app-updates.

implementing a completely safe/atomic IPC lock is probably hard and/or overkill but even a simple implementation could greatly help to prevent the discussed problem.

i haven't put too much thought into it since i want to hear your preferences here first but i suppose we could use either a lockfile or NSDistributedNotification's for this. a lockfile is probably better, unless the 'sandbox' comes into play but i am not sure how many of apps using Sparkle are sandboxed.

we realise that while you may want to see this issue fixed in the interest of users of apps using Sparkle you may not want to put a lot of effort into it yourself. so, we are fully prepared to implement a solution and contribute it to your project. the only thing we would need is
1.) a confirmation beforehand that the contribution will be accepted into Sparkle
2.) some guideline how a solution needs to look to be acceptable
3.) maybe some small pointers where/how best to implement it :)

[1] https://www.corecode.io/macupdater/index.html
[2] which is a totally unbiased statement ^^

[core-code]

note that this also affects the other popular Mac self-update library. i've sent a report to them Squirrel/Squirrel.Mac#245. it would be great if we could solve this situation with a common mechanism to both Sparkle and Squirrel, but we'd have no problem with two distinct solutions either.

[core-code]

@mangerlahn i think you might be interested in seeing a solution to this too :)

[kornelski]

I agree it'd be great to fix it. Being a de-facto standard that is usable by 3rd party apps is one of Sparkle's strengths.

I'm not sure if an event is the right solution, because there needs to be someone actively listening for the event to get it. If the other updater starts after Sparkle sent the event, it may still clash.

A lock may be a better solution. We need to take into account that the app may be killed before it finishes update (e.g. quit during system shutdown, crashes, etc.) so there needs to be a way to unlock a stale lock. Perhaps the pid file pattern would suffice? (the lock would contain pid of the process that is being updated. If such pid doesn't exist, it means the lock is stale and can be ignored).

Apple keeps locking filesystems down, so I'm not sure what filesystem location would be safe long-term. /tmp has a nice property of being cleared after every reboot, so even worst lock mess-ups would be fixable by restarting the system.

Another place that comes to my mind would be NSUserDefaults. It's possible to read other apps' defaults, so Sparkle could set the lock there (and ensure it syncs the defaults to disk).

[core-code]

I agree it'd be great to fix it.

happy to hear that

I'm not sure if an event is the right solution

that was my concern as well. its definitely better than nothing, and has the upside that notifications are unhindered by sandboxing (at least when they don't contain data). but i agree that other solutions are preferable if we can also support a sandbox scenario

Perhaps the pid file pattern would suffice?

i like that idea

Another place that comes to my mind would be NSUserDefaults.

probably a good idea too since NSUserDefaults is naturally concurrency-safe

Apple keeps locking filesystems down

i think its best if i do some testing there to evaluate possible solutions. although i suspect that 99% of Sparkle-using apps are not sandboxed, i guess that we prefer a solution that works fine for sandboxed apps too. i'll have a look at how well a pid-file in /tmp and a token in the user-defaults work for both sandboxed and non-sandboxed apps and come back to you here.

[core-code]

ok i've done some tests.

• Scenario #1: the app using Sparkle is not 'sandboxed':

as expected everything works fine here. obviously both writing/reading files (e.g. to /tmp) and well as NSUserDefaults work fine

• Scenario #2: the app using Sparkle is 'sandboxed':

this is troublesome - also as expected:

• using a file lock in a global location seems impossible. writing to files in /tmp/ definitely does not work (without any special exceptions). as intended by the Sandbox, only locations in the Container can be accessed. [1] the only way to implement a file lock that seems plausible to me would be to use a location inside the Container.

• using a lock in the preferences file is also "problematic". while sandboxed apps can obviously write their preferences, foreign apps cannot trivially access these preferences like is normally done using CFPreferencesCopyAppValue(). i did some tests and 'manually' reading the preferences file stored in the container still works (for the time being - tested on 'Mojave')

---

another thing that i was thinking about it this: concurrent updates may also be going in a multi-user scenario. macOS allows multiple (GUI) users to be logged in simultaneously. i can imagine a scenario where an app updates itself under one account, while being simultaneously being updated in the other account too. i guess this could also happen without the use of any app-updaters, if the Sparkle update is triggered under both accounts at the same time. most of the ideas we've discussed will not help in this scenario (e.g. lock files using the PID or in a Container, NSUserDefaults). however, until we have some evidence that this is an actually occurring problem in the real world, it probably does not make sense to take this into account.

---

i am unsure how important support for Sandboxed apps is to Sparkle - especially since we are talking about a feature here that is not 'essential'.

i guess one decision to make is whether to use a "two way" lock, or just a "simple flag".
what i mean is this:

• a two-way lock would prevent a third-party app-updater to update an app that is already updating itself. but it would also prevent Sparkle from initiating an update when a third-party updater is already doing the update. e.g. the PID-lockfile

• a simpler solution would be just a flag where Sparkle lets the outside world know that an update is in-progress. 3rd party updaters would look at this and leave the app alone.

while the two-way lock seems like the proper solution from a computer-science standpoint, i think the simpler mechanism has a lot going for it:

• it is simpler to make compatible with the Sandbox (if this is a requirement)

• it completely shifts the burden of responsibility to the 3rd party updater. Sparkle would probably only gain two additional lines-of-code that set and remove the flag to let the outside world know that it is updating. its the responsibility of the 3rd party updater not to interfere while this flag is set. this is probably how things should be.

• any downsides are probably mostly theoretical. the theoretical problem with this approach is this: the 3rd party updater might start the update first (because the flag is not set) and then Sparkle starts its update too. i think this scenario is unlikely because 1.) any sensible 3rd party updater will quit an app before updating it and 2.) the only way for an app to update itself while it is not running, is with the silent/automatic updates right after quit. but i guess we can depend on the flag being set after a sensible waiting-time after the app has been quit. i guess waiting for something like 0.5 seconds after quitting the app and if the self-update flag is still not set, then go ahead and update it will be sufficient in almost all cases. since the problem that we want to avoid is (with our current precautions) already pretty rare to begin with, i think this should be more than good enough.

---

Summary: i think we should start with a simple solution here, a flag that informs the outside world that updates are in-progress. if this doesn't turn out to be perfect, we can always revise later on. and this should be very easy to implement.
• if the Sandbox isn't of concern, Sparkle could write a lockfile to /tmp/org.sparkle-project.Sparkle/pid.lock when starting to update an app, and remove it when done
• if support for Sandboxed apps is important or an even simpler approach is desired then i recommend just setting the NSUserDefaults => SUUpdatesInProgress to TRUE while the updates are running. any problems of reading the preferences of sandboxed apps are the problem of outside consumers and shouldn't be Sparkle's concern.

how does that sound? :)

[1] personally i am unsure how how Sandboxed apps are technically able to update themselves using Sparkle anyway. isn't write access to /Applications/ a clear bug in the Sandbox? i know Sparkle takes a great care to make it secure, but my question is, why is it even possible? if Sparkle wasn't that careful it could just replace the app with a non-sandboxed or malicious one so this seems like a definitely a escalation out of the sandbox.

[michelf]

With the 2.x branch, sandboxed apps use an unsandboxed XPC process for the installation step, which then launches the autoupdater (also unsandboxed). Presumably, the XPC or the autoupdater could manage the lock.

[kornelski]

Good point about multi-user access.

For sandboxed applications Sparkle has an XPC service that has privileges to run unsandboxed. It is a huge weakness in the sandbox, but at least the compromised host has to trick the XPC service first.

I think the PID file approach seems most promising. I've just remembered that there's /var/run/ for this purpose and it seems to be used in macOS. So hopefully Apple won't ruin that one too soon :)

So I suggest writing to

/var/run/$hostBundleId.Sparkle.pid

e.g. /var/run/com.example.coolapp.Sparkle.pid that contains 123

Using host's bundle ID, because one app updating itself shouldn't block other apps.

Using separate files, not a directory, because I dread dealing with permissions and potential race conditions in a multi-user environment. All files flat are the simplest.

Appending just Sparkle to app's bundle ID seems sufficient. I don't expect apps to have many things called Sparkle that happen to need to set a PID.

[core-code]

thanks for the explanation regarding how the XPC service works (to both of you). makes sense. and i guess such a setup wouldn't be allowed in the MAS.

So hopefully Apple won't ruin that one too soon :)

hah, nothing seems safe these days :/

So I suggest writing to

thanks for providing a plan how to implement this! i fully agree with your analysis here - best not deal with permissions and folders.

i'll work on an implementation and send a PR once ready.

[michelf]

/var/run/$hostBundleId.Sparkle.pid

Maybe instead of .Sparkle.pid it could use .UpdateInProgress.pid. This way someone looking at the file can understand the purpose even without knowing what "Sparkle" is. Also, this naming convention will be easier to adopt by other auto-updating systems.

[kornelski]

"Sparkle.pid" is so unique, that searching it already points to Sparkle's github, so I think it'll be easy to discover what it does.

[core-code]

ok, sorry it took us a while to come up with an implementation.

as discussed above this can be implemented in various ways, but i think we've settled on a simple solution here for a start.

this has now been provided in #1652

if you are looking for a more complex solution that could also provide safety in case an app is updated just via Sparkle in a multi-user scenario (more likely with automatic updates than with a user clicking 'update' in both GUI accounts nearly simultaneously) then please have a look here: https://github.com/bibhas/Sparkle/compare