766833 - Some MozillaMaintenance installs point to maintenanceservice_tmp.exe

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Brian R. Bondy [:bbondy]]

This bug is to cleanup an old issue that could have happened leaving a few computers in a state where the MozillaMaintenance service points to maintenanceservice_tmp.exe.  There are no fatal problems with this because there is a fallback, but it should be fixed.  It might also be the cause of some intermittent problems with the service. 

The problem can happen if C:\Program Files (x86)\Mozilla Maintenance Service\maintenanceservice.exe exists but the MozillaMaintenance service itself is not installed with Windows.

In that case the maintenanceservice_installer.exe would see that maintenanceservice.exe already exists and it would copy it in as maintenanceservice_tmp.exe. 

serviceinstall.cpp would then try to open the service, but get a "service does not exist" error, so it would install using the current path, which is maintenanceservice_tmp.exe.

The only way I can see this happening is because when we first landed the service, we would uninstall the service and then reinstall on each upgrade. 
There was an intermittent problem that happened on Nightly when we first landed, but it was rare. 

To fix we should detect this in serviceinstall.cpp and fix the problem.

Comment 1

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

Tested locally and it fixes up services installed as _tmp.exe. I'll verify this on oak and do further testing though before asking for review.

Comment 2

[Brian R. Bondy [:bbondy]]

Comment on attachment 635177 [details] [diff] [review]
Patch v1.

This patch is to cleanup anyone that is in a state where their MozillaMaintenance service points to maintenanceservice_tmp.exe.  I think if anyone has this problem it is very very rare, but some test machines have this problem since they ran tests non stop and ran really early builds and pre-release builds.

I tested this patch on Oak with 2 Nightly builds.  It upgrades correctly.  I also manually setup my MozillaMaintenance service to point to maintenanceservice_tmp.exe and tested that it fixes the problem.  When it fixes the problem it will also replace the old maintenanceservice.exe if it needs to be updated (and I also tested that it won't upgrade maintenanceservice.exe when that is already newer, but it will fix that it points to the correct file).

Comment 3

[(no longer active)]

Comment on attachment 635177 [details] [diff] [review]
Patch v1.

Review of attachment 635177 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/maintenanceservice/serviceinstall.cpp
@@ +205,5 @@
> +
> +
> +  if (!ChangeServiceConfig(service, SERVICE_NO_CHANGE, SERVICE_NO_CHANGE,
> +                          SERVICE_NO_CHANGE, fixedPath, NULL, NULL, NULL,
> +                          NULL, NULL, NULL)) {

Nit: ChangeServiceConfigW.

Comment 4

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2.

Thanks for the review, Updated patch with nit.

Comment 5

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/def2b3740f0d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/def2b3740f0d

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 635731 [details] [diff] [review]
Patch v2.

Review of attachment 635731 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/maintenanceservice/serviceinstall.cpp
@@ +165,5 @@
> + * and fixes it if it is wrong.
> + *
> + * @param service            A handle to the service to fix.
> + * @param currentServicePath The current (possibly wrong) path that is used.
> + * @param wasFixNeeded       Out parameter set to TRUE if a fix was needed.

Nit: you actually called it "servicePathWasWrong"

Comment 8

[Brian R. Bondy [:bbondy]]

Oops ya changed it after the original, will update on a follow along, thanks!

Comment 9

[Brian R. Bondy [:bbondy]]

nit was fixed in bug 767471, will land soon.