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

ß72 fixes (+GCD fixes) #1385

Merged
merged 4 commits into from Feb 12, 2013

Conversation

Projects
None yet
3 participants
Owner

pjrobertson commented Feb 11, 2013

Fixes a crash if the input to the AS is nil.

See the comments at #1048

Merge target is release

Use string comparison not pointer comparison for some QSObject iVars
This is important for the identifier since the identifier is used as a key for the objectDictionary dict.
If it happens that the two strings were the same (but not the pointers), then you could potentially remove self from the objectDictionary, and overreleasing it
Owner

pjrobertson commented Feb 11, 2013

Another commit that hopefully fixes a crash from lots of ß72 crash reports:

Crashed Thread:  7  Dispatch queue: QSCatalogEntry scanQueue: QSPresetRemovableVolumes

Exception Type:  EXC_CRASH (SIGABRT)
Exception Codes: 0x0000000000000000, 0x0000000000000000

Application Specific Information:
*** error for object 0x106dcf850: double free


Thread 0:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib          0x00007fff851e4686 mach_msg_trap + 10
1   libsystem_kernel.dylib          0x00007fff851e3c42 mach_msg + 70
2   com.apple.CoreFoundation        0x00007fff8a8e7803 __CFRunLoopServiceMachPort + 195
3   com.apple.CoreFoundation        0x00007fff8a8ecee6 __CFRunLoopRun + 1078
4   com.apple.CoreFoundation        0x00007fff8a8ec6b2 CFRunLoopRunSpecific + 290
5   com.apple.HIToolbox             0x00007fff838600a4 RunCurrentEventLoopInMode + 209
6   com.apple.HIToolbox             0x00007fff8385fe42 ReceiveNextEventCommon + 356
7   com.apple.HIToolbox             0x00007fff8385fcd3 BlockUntilNextEventMatchingListInMode + 62
8   com.apple.AppKit                0x00007fff89d71613 _DPSNextEvent + 685
9   com.apple.AppKit                0x00007fff89d70ed2 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128
10  com.apple.AppKit                0x00007fff89d68283 -[NSApplication run] + 517
11  com.apple.AppKit                0x00007fff89d0ccb6 NSApplicationMain + 869
12  com.blacktree.Quicksilver       0x00000001000018d4 0x100000000 + 6356

Thread 1:: Dispatch queue: com.apple.libdispatch-manager
0   libsystem_kernel.dylib          0x00007fff851e6d16 kevent + 10
1   libdispatch.dylib               0x00007fff8c0e1dea _dispatch_mgr_invoke + 883
2   libdispatch.dylib               0x00007fff8c0e19ee _dispatch_mgr_thread + 54

Thread 2:
0   libsystem_kernel.dylib          0x00007fff851e66d6 __workq_kernreturn + 10
1   libsystem_c.dylib               0x00007fff8be5eeec _pthread_workq_return + 25
2   libsystem_c.dylib               0x00007fff8be5ecb3 _pthread_wqthread + 412
3   libsystem_c.dylib               0x00007fff8be49171 start_wqthread + 13

Thread 3:: Dispatch queue: com.apple.root.low-priority
0   com.apple.CoreFoundation        0x00007fff8a8cf270 CFStringCreateWithBytes + 16
1   com.apple.CoreFoundation        0x00007fff8a8e1f9c _uniqueStringForCharacters + 284
2   com.apple.CoreFoundation        0x00007fff8a952715 parseQuotedPlistString + 917
3   com.apple.CoreFoundation        0x00007fff8a951a03 parsePlistObject + 2659
4   com.apple.CoreFoundation        0x00007fff8a95221f parsePlistDictContent + 1599
5   com.apple.CoreFoundation        0x00007fff8aa1fc79 __CFParseOldStylePropertyListOrStringsFile + 1145
6   com.apple.CoreFoundation        0x00007fff8a9b0fe5 _CFPropertyListCreateFromUTF8Data + 2421
7   com.apple.CoreFoundation        0x00007fff8a8de567 _CFPropertyListCreateWithData + 583
8   com.apple.CoreFoundation        0x00007fff8a8de1a0 CFPropertyListCreateFromXMLData + 160
9   com.apple.Foundation            0x00007fff83fdcd00 +[NSDictionary(NSDictionary) newWithContentsOf:immutable:] + 123
10  com.apple.Foundation            0x00007fff83fdcc6c +[NSDictionary(NSDictionary) dictionaryWithContentsOfFile:] + 43
11  com.blacktree.QSFoundation      0x0000000100053951 -[NSBundle(BLTRExtensions) safeLocalizedStringForKey:value:table:] + 587
12  com.blacktree.QSCore            0x00000001000da92e -[QSCatalogEntry scanForced:] + 509
13  com.blacktree.QSCore            0x00000001000e37ef __42-[QSLibrarian scanCatalogIgnoringIndexes:]_block_invoke + 347
14  libdispatch.dylib               0x00007fff8c0e2f01 _dispatch_call_block_and_release + 15
15  libdispatch.dylib               0x00007fff8c0df0b6 _dispatch_client_callout + 8
16  libdispatch.dylib               0x00007fff8c0e01fa _dispatch_worker_thread2 + 304
17  libsystem_c.dylib               0x00007fff8be5ecab _pthread_wqthread + 404
18  libsystem_c.dylib               0x00007fff8be49171 start_wqthread + 13

Thread 4:
0   libsystem_kernel.dylib          0x00007fff851e66d6 __workq_kernreturn + 10
1   libsystem_c.dylib               0x00007fff8be5eeec _pthread_workq_return + 25
2   libsystem_c.dylib               0x00007fff8be5ecb3 _pthread_wqthread + 412
3   libsystem_c.dylib               0x00007fff8be49171 start_wqthread + 13

Thread 5:
0   libsystem_kernel.dylib          0x00007fff851e6d16 kevent + 10
1   com.blacktree.QSCore            0x00000001001061e4 -[VDKQueue watcherThread:] + 148
2   com.apple.Foundation            0x00007fff84005612 __NSThread__main__ + 1345
3   libsystem_c.dylib               0x00007fff8be5c742 _pthread_start + 327
4   libsystem_c.dylib               0x00007fff8be49181 thread_start + 13

Thread 6:
0   libsystem_kernel.dylib          0x00007fff851e66d6 __workq_kernreturn + 10
1   libsystem_c.dylib               0x00007fff8be5eeec _pthread_workq_return + 25
2   libsystem_c.dylib               0x00007fff8be5ecb3 _pthread_wqthread + 412
3   libsystem_c.dylib               0x00007fff8be49171 start_wqthread + 13

Thread 7 Crashed:: Dispatch queue: QSCatalogEntry scanQueue: QSPresetRemovableVolumes
0   libsystem_kernel.dylib          0x00007fff851e6212 __pthread_kill + 10
1   libsystem_c.dylib               0x00007fff8be5daf4 pthread_kill + 90
2   libsystem_c.dylib               0x00007fff8bea1dce abort + 143
3   libsystem_c.dylib               0x00007fff8be7d8a5 szone_error + 580
4   libsystem_c.dylib               0x00007fff8be75898 free + 199
5   com.apple.CoreFoundation        0x00007fff8a9debe9 __rehashd + 361
6   com.apple.CoreFoundation        0x00007fff8a9d99ec -[__NSDictionaryM setObject:forKey:] + 476
7   com.blacktree.QSCore            0x00000001000e7716 -[QSObject(Accessors) setIdentifier:] + 114
8   com.blacktree.QSCore            0x00000001000e7699 -[QSObject(Accessors) identifier] + 147
9   com.blacktree.QSCore            0x00000001000e5bca -[QSObject isEqual:] + 160
10  com.apple.CoreFoundation        0x00007fff8a9e3804 -[__NSSetM addObject:] + 388
11  com.apple.CoreFoundation        0x00007fff8a928349 -[NSMutableSet addObjectsFromArray:] + 473
12  com.blacktree.QSCore            0x00000001000e23fb -[QSLibrarian reloadSets:] + 300
13  com.apple.CoreFoundation        0x00007fff8a8fd47a _CFXNotificationPost + 2554
14  com.apple.Foundation            0x00007fff83fb8846 -[NSNotificationCenter postNotificationName:object:userInfo:] + 64
15  com.blacktree.QSCore            0x00000001000da6b1 __30-[QSCatalogEntry scanAndCache]_block_invoke + 355
16  libdispatch.dylib               0x00007fff8c0df0b6 _dispatch_client_callout + 8
17  libdispatch.dylib               0x00007fff8c0e0723 _dispatch_barrier_sync_f_invoke + 39
18  com.blacktree.QSCore            0x00000001000da4db -[QSCatalogEntry scanAndCache] + 177
19  com.blacktree.QSCore            0x00000001000e34bd -[QSLibrarian loadMissingIndexes] + 249
20  com.blacktree.Quicksilver       0x0000000100007b94 0x100000000 + 31636
21  com.apple.Foundation            0x00007fff84005612 __NSThread__main__ + 1345
22  libsystem_c.dylib               0x00007fff8be5c742 _pthread_start + 327
23  libsystem_c.dylib               0x00007fff8be49181 thread_start + 13
Owner

pjrobertson commented Feb 11, 2013

OK, and another commit which hopefully fixes the the issues over at #1367

I'll try and debug it a bit more with Phil

OK, but what about the case where they are the same object and the current retain count happens to be 1?

[thing release];
thing = [newThing retain];

Won't that crash on the second line since the object was just freed? Do we need to test both string and object equality?

Owner

pjrobertson replied Feb 12, 2013

Then if they are the same, then the code is skipped and the release/retain is never called. :)

Duh. :-)

One more question: Do we need to do this for name and label? It makes sense, but all of the examples I've seen compare the objects. (I assume because it's much faster than string comparisons.)

@skurfer skurfer commented on the diff Feb 12, 2013

...ilver/Code-QuickStepInterface/QSInterfaceController.m
@@ -642,10 +642,16 @@ - (void)executeCommand:(id)sender cont:(BOOL)cont encapsulate:(BOOL)encapsulate
if (!cont) {
[self hideMainWindowFromExecution:self]; // *** this should only hide if no result comes in like 2 seconds
}
- if ([[NSUserDefaults standardUserDefaults] boolForKey:kExecuteInThread] && [[aSelector objectValue] canThread])
- [NSThread detachNewThreadSelector:@selector(executeCommandThreaded) toTarget:self withObject:nil];
- else
- [self executeCommandThreaded];
+ if ([[NSUserDefaults standardUserDefaults] boolForKey:kExecuteInThread] && [action canThread]) {
+ dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^{
+ [self executeCommandThreaded];
+ });
+ } else {
+ // action can only be run on main thread
+ runOnMainQueueSync(^{
@skurfer

skurfer Feb 12, 2013

Owner

Why not just call the method like it did before? I'm sure there's a reason, I just don't know what it is. :-)

@pjrobertson

pjrobertson Feb 12, 2013

Owner

It was just to make sure the current thread was definitely the main thread. The only way didn't necessarily guarantee it would be (Except that executeCommand: is always called on the main thread.

I thought it could flake in the future, say if we decide executeCommand should all be done in the background (which it probably should...!)

Owner

skurfer commented Feb 12, 2013

OK, then this all seems OK to me. I'll test for a few hours (and give @tiennou time to object) before doing another release.

@tiennou tiennou commented on an outdated diff Feb 12, 2013

Quicksilver/Code-App/QSController.m
@@ -844,7 +839,11 @@ - (void)applicationWillFinishLaunching:(NSNotification *)aNotification {
- (void)setupSplash {
if ([[NSUserDefaults standardUserDefaults] boolForKey:kUseEffects]) {
[self showSplash:nil];
- [NSTimer scheduledTimerWithTimeInterval:0.1 target:self selector:@selector(threadedHideSplash) userInfo:nil repeats:NO];
+ double delayInSeconds = 0.1;
+ dispatch_time_t popTime = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(delayInSeconds * NSEC_PER_SEC));
+ dispatch_after(popTime, dispatch_get_main_queue(), ^(void){
@tiennou

tiennou Feb 12, 2013

Owner

You're doing that...

@tiennou tiennou commented on an outdated diff Feb 12, 2013

Quicksilver/Code-App/QSController.m
@@ -1053,7 +1053,9 @@ - (void)setActivationHotKey:(id)object {
}
- (void)threadedHideSplash {
- [NSThread detachNewThreadSelector:@selector(hideSplash:) toTarget:self withObject:nil];
+ dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
@tiennou

tiennou Feb 12, 2013

Owner

... and then you're setting it to run on a background queue. Might be better to call it directly (and remove threadedHideSplash.

pjrobertson added a commit that referenced this pull request Feb 12, 2013

Merge pull request #1385 from pjrobertson/release
ß72 fixes (GCD Etienne! :D )

@pjrobertson pjrobertson merged commit 04730af into quicksilver:release Feb 12, 2013

@pjrobertson pjrobertson deleted the pjrobertson:release branch Feb 12, 2013

skurfer added a commit that referenced this pull request Feb 13, 2013

increase build number to 3945
mention #1385 in the release notes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment