Closed Bug 1286490 Opened 4 years ago Closed 3 years ago

[Elevated Update] Fast Elevation Cancel hangs Firefox on Mac OS X

Categories

(Toolkit :: Application Update, defect)

All
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla51
Tracking Status
firefox49 --- verified
firefox50 --- verified
firefox51 --- verified

People

(Reporter: bogdan_maris, Assigned: spohl)

References

Details

Attachments

(3 files)

[Note]:
- This is a followup of bug 1274319

[Affected versions]:
- Developer Edition 49.0a2
- Nightly 50.0a1

[Affected platforms]:
- Mac OS X 10.9

[Steps to reproduce]:
1. Install DevEdition from a few days ago from Admin account.
2. Switch to standard account.
3. Check for update, download and restart to update.
4. When the elevated authentication window is displayed press a fast Cancel.

If Firefox starts repeat steps 3-4. (I usually reproduce when I cancel the second time).

[Expected result]:
- Firefox restarts after canceling the login window.

[Actual result]:
- Firefox hangs after canceling the login window.

[Regression range]:
- This is not a regression, it reproduced on old builds as well and bug 1274319 never fixed it on 10.9.

[Additional notes]:
- Message I get from terminal, when hang occurs:

> sl115-32:~ standard1$ 2016-07-11 11:43:15.013 firefox[715:303] AuthorizationCopyRights failed! NSOSStatusErrorDomain / -60006
> 2016-07-11 11:43:15.032 firefox[715:303] Unable to clean up updater.
When Firefox is in the bad state, could you run the following three commands and tell me what gets printed to the console for each one:

sudo launchctl list | grep org.mozilla.updater
sudo ps ax | grep org.mozilla.updater
sudo ps ax | grep firefox

Thanks!
Flags: needinfo?(bogdan.maris)
(In reply to Stephen A Pohl [:spohl] from comment #1)
> When Firefox is in the bad state, could you run the following three commands
> and tell me what gets printed to the console for each one:
> 
> sudo launchctl list | grep org.mozilla.updater
> sudo ps ax | grep org.mozilla.updater
> sudo ps ax | grep firefox
> 
> Thanks!

Sure, here is the terminal output: http://pastebin.com/XiFXs93P
Not sure if this helps or not.
Flags: needinfo?(bogdan.maris) → needinfo?(spohl.mozilla.bugs)
Blocks: 394984
Depends on: 1274319
Attached patch Improve loggingSplinter Review
The timestamps in comment 0 between "AuthorizationCopyRights failed" and "Unable to clean up updater!" are in very fast succession, which seems to indicate that we hit an NSException and we don't actually retry the cleanup attemp ten times. Before I rewrite the try/catch block to retry even when we hit an NSException, I'd like to see what kind of exception we're actually hitting. I'll fix the actual issue in a subsequent patch once we know more.
Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED
Flags: needinfo?(spohl.mozilla.bugs)
Attachment #8780598 - Flags: review?(mstange)
Attachment #8780598 - Flags: review?(mstange) → review+
Setting checking-needed since inbound is currently closed.
Please retest this after the patch has been on nightly for a couple of days. Once the issue reproduces, please copy the console output from Terminal or Console in a new comment like you did in comment 0. Thanks!
Flags: needinfo?(bogdan.maris)
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/9b1f6f45c1d7
Improve logging when unable to clean up elevated updater on OSX. r=mstange
Keywords: checkin-needed
Here is the output of the Terminal while fast canceling an update from Nightly (2016-08-23) using Mac OS X 10.9.5, 10.10.5 and 10.11.1 (I reproduced on all of them).

> SL115-136:~ standard$ /Applications/FirefoxNightly.app/Contents/MacOS/firefox -no-remote -jsconsole
> 
> 2016-08-25 17:25:34.216 firefox[2671:141946] AuthorizationCopyRights failed! NSOSStatusErrorDomain / -60006
> SL115-136:~ standard$ 2016-08-25 17:25:51.583 firefox[2685:142488] AuthorizationCopyRights failed! NSOSStatusErrorDomain / -60006
> 
> 2016-08-25 17:25:51.605 firefox[2685:142488] NSPortTimeoutException: Distributed objects message send timed out (timeout: 10493827951.594898 at time: 493827951.595064) 1
> 
> 2016-08-25 17:25:51.605 firefox[2685:142488] Unable to clean up updater.
Flags: needinfo?(bogdan.maris) → needinfo?(spohl.mozilla.bugs)
Attached patch PatchSplinter Review
Ok, this patch should take care of it. Thanks!
Flags: needinfo?(spohl.mozilla.bugs)
Attachment #8784960 - Flags: review?(mstange)
Attachment #8784960 - Flags: review?(mstange) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/435841ba617b50c2c59296694db1103e0d0debb6
Bug 1286490: Handle IPC timeout exceptions during elevated updates on OSX. r=mstange
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/435841ba617b
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Bogdan, could you verify that this is indeed fixed now? Thanks!
Flags: needinfo?(bogdan.maris)
Just verified on Mac OS X 10.10.5, 10.11.1 and 10.9.5 canceling the update from old Nightly (2016-08-27) to latest Nightly did not hang Firefox no matter how many times I canceled the update. 

Will this be uplifted to beta as well? It could be nice to have this in 49 when it reaches release.
Flags: needinfo?(bogdan.maris)
Attached patch Patch for upliftSplinter Review
Thanks, Bogdan! Not sure if we can uplift all the way, but I can certainly request it and see.

Approval Request Comment
[Feature/regressing bug #]: Bug 394984
[User impact if declined]: Fast cancellations of the elevation prompt during an elevated update on OSX can hang Firefox.
[Describe test coverage new/current, TreeHerder]: Verified by QA.
[Risks and why]: Minimal to none. Very isolated and well understood patch.
[String/UUID change made/needed]: none
Attachment #8785960 - Flags: review+
Attachment #8785960 - Flags: approval-mozilla-beta?
Attachment #8785960 - Flags: approval-mozilla-aurora?
Status: RESOLVED → VERIFIED
Comment on attachment 8785960 [details] [diff] [review]
Patch for uplift

Fixes a hang in a corner case, fix was verified on Nightly, Aurora50+
Attachment #8785960 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8785960 [details] [diff] [review]
Patch for uplift

Maybe an edge case but we verified the fix and I'd like to make sure as many people can update smoothly as possible.  Taking this for beta 9.
Attachment #8785960 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
We should verify this on 49.0b9 as well. Bogdan, could you please have a look?
Flags: needinfo?(bogdan.maris)
QA [:avaida] – please ni? me from comment #19)
> We should verify this on 49.0b9 as well. Bogdan, could you please have a
> look?

Sure. I just finished verifying this across different Mac versions (10.11.1, 10.10.5 and 10.9.5) using tinderbox mozilla-beta build and updated to latest Firefox 49 beta 9 using 'beta-cdntest' update chanel. I'll mark this as verified in 50 as well since I tested on Aurora as well a few weeks ago.(In reply to Andrei Vaida,
Flags: needinfo?(bogdan.maris)
Summary: [Elevated Update] Fast Elevation Cancel hangs Firefox on Mac OS X 10.9 → [Elevated Update] Fast Elevation Cancel hangs Firefox on Mac OS X
You need to log in before you can comment on or make changes to this bug.