Recent #1051 crashes the updater with EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)

[Thesaurus]

To me it seems that the recent #1051 pull request merge crashes my app.
I will post a stack trace tomorrow but for now I would be wary of this commit.

[Thesaurus]

EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)

Stack trace is as follows.
Crash occurs on 10.12.5 when NSURLDownloadDelegate method gets called when no update is available:

#0 0x0000000101ed6266 in dispatch_barrier_sync_f_slow ()
#1 0x0000000100bcaf2e in -[SUUIBasedUpdateDriver showAlert:] at SUUIBasedUpdateDriver.m:331
#2 0x0000000100bcae9d in -[SUUIBasedUpdateDriver showAlert:] at SUUIBasedUpdateDriver.m:326
#3 0x0000000100bc9085 in -[SUUIBasedUpdateDriver didNotFindUpdate] at SUUIBasedUpdateDriver.m:118
#4 0x0000000100bb0045 in -[SUBasicUpdateDriver appcastDidFinishLoading:] at SUBasicUpdateDriver.m:198
#5 0x0000000100bd3aec in -[SUUserInitiatedUpdateDriver appcastDidFinishLoading:] at SUUserInitiatedUpdateDriver.m:68
#6 0x0000000100ba9cbb in -[SUAppcast downloadDidFinish:] at SUAppcast.m:110
#7 0x00007fff96d21d5d in -[NSURLDownload sendDidFinish] ()
#8 0x00007fff96d230d0 in _NSURLDownloadDidFinish(_CFURLDownload*, void const*) ()
#9 0x00007fff96c40a5f in URLDownload::_internal_downloadFinished() ()
#10 0x00007fff96c42f0a in ___ZN11URLDownload30_internal_withClientSchedulingEU13block_pointerFvP22CFURLDownloadClient_V2E_block_invoke ()
#11 0x0000000101eca78c in _dispatch_client_callout ()
#12 0x0000000101edf935 in _dispatch_block_invoke_direct ()
#13 0x00007fff96afa018 in RunloopBlockContext::_invoke_block(void const*, void*) ()
#14 0x00007fff97959e34 in CFArrayApplyFunction ()
#15 0x00007fff96af9f11 in RunloopBlockContext::perform() ()
#16 0x00007fff96af9daa in MultiplexerSource::perform() ()
#17 0x00007fff96af9bcc in MultiplexerSource::_perform(void*) ()
#18 0x00007fff979b7321 in CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION ()

• ShowAlert was modified to use dispatch_sync(dispatch_get_main_queue(), ^{ and this call is the case of the crash. Even passing a no-op block crashes. I can only presume that there is some sort of dispatch issue at play here (there are several other dispatch calls higher up in the stack trace). I use the same dispatch_sync() approach myself elsewhere in my code and all is good.

Wanting to ensure that the alert runs on the main thread is a good idea and refactoring to use the older -performSelectorOnMainThread: approach seems to work. However this commit contains other dispatch based patches that I have not looked at and which may exhibit the same behaviour.

Personally I would consider #1051 toxic and revert it to allow the original contributor to rework it.

[kornelski]

Thanks for the report. I've reverted the commit.