1335916 - Crash when staging an update using a local build

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Molly Howell (she/her) (no longer active)]

I'm trying to run the updater tests using a build I made on my macOS Sierra (10.12.3) machine, and the tests that stage updates consistently crash with the following exception:

"2017-02-01 13:39:04.139 xpcshell[68513:687920] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'nextEventMatchingMask should only be called from the Main Thread!'"
"*** First throw call stack:"
"("
"	0   CoreFoundation                      0x00007fff818dbe7b __exceptionPreprocess + 171"
"	1   libobjc.A.dylib                     0x00007fff964c5cad objc_exception_throw + 48"
"	2   AppKit                              0x00007fff7fafe3fd -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 4471"
"	3   XUL                                 0x000000010d09b004 -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116"
"	4   XUL                                 0x000000010e6fce39 _Z31ProcessPendingGetURLAppleEventsv + 89"
"	5   XUL                                 0x000000010e6ebe03 _ZN21CommandLineServiceMac19SetupMacCommandLineERiRPPcb + 243"
"	6   XUL                                 0x000000010e6edd3a _ZL11ApplyUpdateP7nsIFileS0_S0_S0_iPPcbbS0_Pi + 1994"
"	7   XUL                                 0x000000010e6ed07b _Z14ProcessUpdatesP7nsIFileS0_S0_iPPcPKcbbS0_Pi + 1099"
"	8   XUL                                 0x000000010e6f1893 _ZN17nsUpdateProcessor17StartStagedUpdateEv + 403"

[the rest of the stack isn't relevant]

Commenting out the call to ProcessPendingGetURLAppleEvents in nsCommandLineServiceMac.cpp stops the crash happening without any ill effect on the tests, but probably isn't otherwise a good idea.

sUpdateProcessor::StartStagedUpdate() asserts that it *isn't* called from the main thread, and has for a long time, but this crash hasn't been happening for more than a few months (I'm not sure exactly).

Since this isn't happening in official builds even when running on Sierra, and it wasn't happening to me either a few months ago, my hunch is that there's been a change in the macOS SDK that's causing this problem for builds made on Sierra, but I have no way to check that theory.

Comment 1

[Molly Howell (she/her) (no longer active)]

Stephen, would you mind providing some Mac expertise? I don't really understand what happened here.

Comment 2

[Stephen A Pohl [:spohl]]

It is possible that this is another case where Apple is cracking down on incorrect uses of their APIs. Unfortunately, afaict the documentation doesn't make it clear that nextEventMatchingMask should only be called from the main thread. We may have to dispatch this to the main thread via a runnable method like we do here: https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/nsUpdateDriver.cpp#1280

Comment 3

[Molly Howell (she/her) (no longer active)]

Attachment: Bug 1335916 - Make sure the update driver only calls SetupMacCommandLine from the main thread.

Review commit: https://reviewboard.mozilla.org/r/109326/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109326/

Comment 4

[Robert Strong (they/them - no direct email)]

Comment on attachment 8833104 [details]
Bug 1335916 - Make sure the update driver only calls SetupMacCommandLine from the main thread.

https://reviewboard.mozilla.org/r/109326/#review112760

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/27ed205d7628
Make sure the update driver only calls SetupMacCommandLine from the main thread. r=rstrong

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/27ed205d7628