Skip to content
New issue

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

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix crash caused by race condition per @peternlewis #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix crash caused by race condition per @peternlewis #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions VDKQueue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ extern NSString * VDKQueueAccessRevocationNotification;
@private
int _coreQueueFD; // The actual kqueue ID (Unix file descriptor).
NSMutableDictionary *_watchedPathEntries; // List of VDKQueuePathEntries. Keys are NSStrings of the path that each VDKQueuePathEntry is for.
NSMutableDictionary *_pathMap; // unique id -> path entry (for thread safety)
BOOL _keepWatcherThreadRunning; // Set to NO to cancel the thread that watches _coreQueueFD for kQueue events
}

Expand Down
29 changes: 17 additions & 12 deletions VDKQueue.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ - (id) initWithPath:(NSString*)inPath andSubscriptionFlags:(u_int)flags;
@property (atomic, copy) NSString *path;
@property (atomic, assign) int watchedFD;
@property (atomic, assign) u_int subscriptionFlags;
@property (atomic, assign) uintptr_t uniqueId;

@end

Expand Down Expand Up @@ -134,6 +135,7 @@ - (id) init

_alwaysPostNotifications = NO;
_watchedPathEntries = [[NSMutableDictionary alloc] init];
_pathMap = [[NSMutableDictionary alloc] init];
}
return self;
}
Expand All @@ -149,6 +151,7 @@ - (void) dealloc

[_watchedPathEntries release];
_watchedPathEntries = nil;
[_pathMap release];

[super dealloc];
}
Expand All @@ -162,6 +165,7 @@ - (void) dealloc

- (VDKQueuePathEntry *) addPathToQueue:(NSString *)path notifyingAbout:(u_int)flags
{
static uintptr_t gUniqueID = 0;
@synchronized(self)
{
// Are we already watching this path?
Expand All @@ -188,9 +192,11 @@ - (VDKQueuePathEntry *) addPathToQueue:(NSString *)path notifyingAbout:(u_int)fl

if (pathEntry)
{
EV_SET(&ev, [pathEntry watchedFD], EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_CLEAR, flags, 0, pathEntry);
EV_SET(&ev, [pathEntry watchedFD], EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_CLEAR, flags, 0, (void *)gUniqueID);

[pathEntry setSubscriptionFlags:flags];
[pathEntry setUniqueId:gUniqueID];
[_pathMap setObject:pathEntry forKey:@(gUniqueID++)];

[_watchedPathEntries setObject:pathEntry forKey:path];
kevent(_coreQueueFD, &ev, 1, NULL, 0, &nullts);
Expand Down Expand Up @@ -242,18 +248,15 @@ - (void) watcherThread:(id)sender
{
//NSLog( @"KEVENT flags are set" );

//
// Note: VDKQueue gets tested by thousands of CodeKit users who each watch several thousand files at once.
// I was receiving about 3 EXC_BAD_ACCESS (SIGSEGV) crash reports a month that listed the 'path' objc_msgSend
// as the culprit. That suggests the KEVENT is being sent back to us with a udata value that is NOT what we assigned
// to the queue, though I don't know why and I don't know why it's intermittent. Regardless, I've added an extra
// check here to try to eliminate this (infrequent) problem. In theory, a KEVENT that does not have a VDKQueuePathEntry
// object attached as the udata parameter is not an event we registered for, so we should not be "missing" any events. In theory.
//
id pe = ev.udata;
if (pe && [pe respondsToSelector:@selector(path)])
uintptr_t uid = (uintptr_t)ev.udata;
VDKQueuePathEntry *pe;
@synchronized(self)
{
NSString *fpath = [((VDKQueuePathEntry *)pe).path retain]; // Need to retain so it does not disappear while the block at the bottom is waiting to run on the main thread. Released in that block.
pe = [_pathMap objectForKey:@(uid)];
}
if (pe)
{
NSString *fpath = [pe.path retain]; // Need to retain so it does not disappear while the block at the bottom is waiting to run on the main thread. Released in that block.
if (!fpath) continue;

[[NSWorkspace sharedWorkspace] noteFileSystemChanged:fpath];
Expand Down Expand Up @@ -405,6 +408,7 @@ - (void) removePath:(NSString *)aPath

// Remove it only if we're watching it.
if (entry) {
[_pathMap removeObjectForKey:@(entry.uniqueId)];
[_watchedPathEntries removeObjectForKey:aPath];
}
}
Expand All @@ -417,6 +421,7 @@ - (void) removeAllPaths
{
@synchronized(self)
{
[_pathMap removeAllObjects];
[_watchedPathEntries removeAllObjects];
}
}
Expand Down