Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Copy `entryContents` before adding it to an array #1278

Merged
merged 1 commit into from Dec 12, 2012

Conversation

Projects
None yet
2 participants
Owner

pjrobertson commented Dec 12, 2012

Avoid the chance that the object will be released before being added to the array.

You knew that we hadn't actually fixed the catalog scanning crashes, right? ;-)

This fix is in relation to the following crash report:
I recently put a lock on entryContents, so I'm pretty sure that this crash is unrelated to threading problems in the reloadSets method (yes, I did just say that!)
If you look at thread 9, you'll see that the mutex lock (presumably on catalog) is working.

What I think the problem here is, is that entryContents must sill be getting modified somewhere before it's added to the array (released, most likely). I've taken what you could perhaps call a shortcut in avoiding any threading problems and just copying the object.

Note: This pull request is against release. We need to get another release build out anyway so that we can put new localisation stuff in

Process:         Quicksilver [20455]
Path:            /Applications/Quicksilver.app/Contents/MacOS/Quicksilver
Identifier:      com.blacktree.Quicksilver
Version:         ß71 (3937)
Code Type:       X86-64 (Native)
Parent Process:  ??? [1]

Date/Time:       2012-12-11 23:57:44.798 +0100
OS Version:      Mac OS X 10.7.5 (11G63)
Report Version:  9

Crashed Thread:  0  Dispatch queue: QSPresetRecentApplications106

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: 0x000000000000000d, 0x0000000000000000

VM Regions Near 0:
--> 
    __TEXT                 0000000100000000-0000000100031000 [  196K] r-x/rwx SM=COW  /Applications/Quicksilver.app/Contents/MacOS/Quicksilver

Application Specific Information:
objc[20455]: garbage collection is OFF

Thread 0 Crashed:: Dispatch queue: QSPresetRecentApplications106
0   com.apple.CoreFoundation        0x00007fff8ef1642c CFHash + 332
1   com.apple.CoreFoundation        0x00007fff8ef1f605 CFBasicHashAddValue + 1573
2   com.apple.CoreFoundation        0x00007fff8ef879a1 -[NSMutableSet addObjectsFromArray:] + 513
3   com.blacktree.QSCore            0x00000001000dc9d4 -[QSLibrarian reloadSets:] + 323
4   com.apple.Foundation            0x00007fff86fedd0e __-[NSNotificationCenter addObserver:selector:name:object:]_block_invoke_1 + 47
5   com.apple.CoreFoundation        0x00007fff8ef577ba _CFXNotificationPost + 2634
6   com.apple.Foundation            0x00007fff86fd9fc3 -[NSNotificationCenter postNotificationName:object:userInfo:] + 65
7   com.blacktree.QSCore            0x00000001000d51b5 __30-[QSCatalogEntry scanAndCache]_block_invoke_0 + 359
8   libdispatch.dylib               0x00007fff8a86bc75 _dispatch_barrier_sync_f_invoke + 33
9   com.blacktree.QSCore            0x00000001000d4ffe -[QSCatalogEntry scanAndCache] + 181
10  com.blacktree.QSCore            0x00000001000d5525 -[QSCatalogEntry scanForced:] + 756
11  com.apple.Foundation            0x00007fff86fedd0e __-[NSNotificationCenter addObserver:selector:name:object:]_block_invoke_1 + 47
12  com.apple.CoreFoundation        0x00007fff8ef577ba _CFXNotificationPost + 2634
13  com.apple.Foundation            0x00007fff86fd9fc3 -[NSNotificationCenter postNotificationName:object:userInfo:] + 65
14  com.blacktree.QSCore            0x00000001000feeda __26-[VDKQueue watcherThread:]_block_invoke_0 + 223
15  libdispatch.dylib               0x00007fff8a869a82 _dispatch_call_block_and_release + 18
16  libdispatch.dylib               0x00007fff8a86b8f2 _dispatch_main_queue_callback_4CF + 308
17  com.apple.CoreFoundation        0x00007fff8ef43e7c __CFRunLoopRun + 1724
18  com.apple.CoreFoundation        0x00007fff8ef43486 CFRunLoopRunSpecific + 230
19  com.apple.HIToolbox             0x00007fff866a72bf RunCurrentEventLoopInMode + 277
20  com.apple.HIToolbox             0x00007fff866ae4bf ReceiveNextEventCommon + 181
21  com.apple.HIToolbox             0x00007fff866ae3fa BlockUntilNextEventMatchingListInMode + 62
22  com.apple.AppKit                0x00007fff874be779 _DPSNextEvent + 659
23  com.apple.AppKit                0x00007fff874be07d -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 135
24  com.apple.AppKit                0x00007fff874ba9b9 -[NSApplication run] + 470
25  com.apple.AppKit                0x00007fff87736eac NSApplicationMain + 867
26  com.blacktree.Quicksilver       0x0000000100002224 0x100000000 + 8740

...

Thread 8:: Dispatch queue: QSPresetQSCatalogEntries
0   libsystem_kernel.dylib          0x00007fff88b41bf2 __psynch_mutexwait + 10
1   libsystem_c.dylib               0x00007fff846a31a1 pthread_mutex_lock + 545
2   com.blacktree.QSCore            0x00000001000dc8ef -[QSLibrarian reloadSets:] + 94
3   com.apple.Foundation            0x00007fff86fedd0e __-[NSNotificationCenter addObserver:selector:name:object:]_block_invoke_1 + 47
4   com.apple.CoreFoundation        0x00007fff8ef577ba _CFXNotificationPost + 2634
5   com.apple.Foundation            0x00007fff86fd9fc3 -[NSNotificationCenter postNotificationName:object:userInfo:] + 65
6   com.blacktree.QSCore            0x00000001000d51b5 __30-[QSCatalogEntry scanAndCache]_block_invoke_0 + 359
7   libdispatch.dylib               0x00007fff8a86bc75 _dispatch_barrier_sync_f_invoke + 33
8   com.blacktree.QSCore            0x00000001000d4ffe -[QSCatalogEntry scanAndCache] + 181
9   com.blacktree.QSCore            0x00000001000dd94a -[QSLibrarian loadMissingIndexes] + 245
10  com.blacktree.Quicksilver       0x000000010000830a 0x100000000 + 33546
11  com.apple.Foundation            0x00007fff8703172a -[NSThread main] + 68
12  com.apple.Foundation            0x00007fff870316a2 __NSThread__main__ + 1575
13  libsystem_c.dylib               0x00007fff846a48bf _pthread_start + 335
14  libsystem_c.dylib               0x00007fff846a7b75 thread_start + 13

Thread 0 crashed with X86 Thread State (64-bit):
  rax: 0x0000000000000000  rbx: 0xb0000000105783c0  rcx: 0x00007fff74cdade8  rdx: 0xb0000000105783c0
  rdi: 0xb0000000105783c0  rsi: 0xb0000000105783c0  rbp: 0x00007fff5fbfdf00  rsp: 0x00007fff5fbfdee0
   r8: 0x0000000000000000   r9: 0x0000000000000004  r10: 0x0000000100235030  r11: 0x00000001002101d8
  r12: 0x0000000000000048  r13: 0x0000000000000001  r14: 0x0000000000000000  r15: 0x00007fff74cd9520
  rip: 0x00007fff8ef1642c  rfl: 0x0000000000010246  cr2: 0x000000015de3f000
Logical CPU: 2
Copy `entryContents` before adding it to an array
Avoid the chance that the object will be released/modified before being added to the array.
Owner

skurfer commented Dec 12, 2012

You knew that we hadn't actually fixed the catalog scanning crashes, right? ;-)

Of course. :-) I think this will also fix the crash I reported on the initial pull request. I'll try it out. Are you going to do a separate pull request for the updated localizations?

Owner

pjrobertson commented Dec 12, 2012

Are you going to do a separate pull request for the updated localizations?

Oh yeah, forgot we were going to do pull requests for localisations. I'll
get one out

On 12 December 2012 15:21, Rob McBroom notifications@github.com wrote:

Are you going to do a separate pull request for the updated localizations?

skurfer added a commit that referenced this pull request Dec 12, 2012

Merge pull request #1278 from pjrobertson/reloadSetsFix
Copy `entryContents` before adding it to an array

@skurfer skurfer merged commit 516d8ae into quicksilver:release Dec 12, 2012

skurfer added a commit that referenced this pull request Dec 12, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment