search

karelia / CurlHandle

Cocoa Class wrapping libcurl
54 stars 21 forks source link

Crash on Sierra? #23

Closed MrNoodle closed 7 years ago

MrNoodle commented 7 years ago

Unable to replicate this myself but have had a couple reports from users with the following excerpted dump (the stack is nearly identical in each case). Note that I'm using ConnectionKit and not CurlHandle directly. If I should post this issue there instead, let me know.

Crashed Thread: 3 Dispatch queue: com.karelia.CURLMulti.0x608000098a10

Exception Type: EXC_BAD_INSTRUCTION (SIGILL) Exception Codes: 0x0000000000000001, 0x0000000000000000 Exception Note: EXC_CORPSE_NOTIFY

Termination Signal: Illegal instruction: 4 Termination Reason: Namespace SIGNAL, Code 0x4 Terminating Process: exc handler [0]

Application Specific Information: BUG IN CLIENT OF LIBDISPATCH: dispatch_barrier_sync called on queue already owned by current thread

[...stuff omitted...]

Thread 3 Crashed:: Dispatch queue: com.karelia.CURLMulti.0x608000098a10 0 libdispatch.dylib 0x00007fffb74916fb _dispatch_barrier_sync_f_slow + 675 1 com.karelia.CURLHandle 0x000000010d410e39 -[CURLTransferStack cleanupMulti] + 86 2 libdispatch.dylib 0x00007fffb748ef5f _dispatch_call_block_and_release + 12 3 libdispatch.dylib 0x00007fffb7486128 _dispatch_client_callout + 8 4 libdispatch.dylib 0x00007fffb749cb97 _dispatch_queue_serial_drain + 896 5 libdispatch.dylib 0x00007fffb748ed41 _dispatch_queue_invoke + 1046 6 libdispatch.dylib 0x00007fffb7495158 _dispatch_queue_override_invoke + 369 7 libdispatch.dylib 0x00007fffb7487ee0 _dispatch_root_queue_drain + 476 8 libdispatch.dylib 0x00007fffb7487cb7 _dispatch_worker_thread3 + 99 9 libsystem_pthread.dylib 0x00007fffb76d2746 _pthread_wqthread + 1299 10 libsystem_pthread.dylib 0x00007fffb76d2221 start_wqthread + 13

MrNoodle commented 7 years ago

Looking at it more closely, I'm guessing what used to be a deadlock is now detected and an exception is thrown instead. Seems that -cleanupMulti, which does a dispatch_sync() is being called indirectly via -setState: from various dispatch_async() blocks.

MrNoodle commented 7 years ago

So, with my limited knowledge of the code, I'm thinking that the dispatch_sync() call in -cleanupMulti can be removed. Everywhere that I can tell that it's called, it's being called on the queue (either via dispatch_async() or via a dispatch_source cancel handler. If that seems correct to you then I can make the change (or you could do it yourself since it's pretty easy).

andypotion commented 7 years ago

@MrNoodle is right. Code was deadlocking before but Sierra throws out an exception. The fix was to either get rid of the dispatch_sync call or change it to dispatch_async. I've gone with the latter to be more cautious in case there were other things still in the queue. Fix has been pushed to the v4.x-beta branch