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.

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

fix crash calling nonexistent delegate (also memory leak) #20

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion VDKQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
//
// VDKQueue is also simplified. The option to use it as a singleton is removed. You simply alloc/init an instance and add paths you want to
// watch. Your objects can be alerted to changes either by notifications or by a delegate method (or both). See below.
// When you're done with the object, call stopWatching (to release the background thread), then release.
//
// It also fixes several bugs. For one, it won't crash if it can't create a file descriptor to a file you ask it to watch. (By default, an OS X process can only
// have about 3,000 file descriptors open at once. If you hit that limit, UKKQueue will crash. VDKQueue will not.)
Expand Down Expand Up @@ -123,7 +124,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.
BOOL _keepWatcherThreadRunning; // Set to NO to cancel the thread that watches _coreQueueFD for kQueue events
BOOL _keepWatcherThreadRunning; // Keeps track of whether the thread is running or not.
}


Expand All @@ -139,6 +140,7 @@ extern NSString * VDKQueueAccessRevocationNotification;
- (void) removePath:(NSString *)aPath;
- (void) removeAllPaths;

- (void) stopWatching; // You must call this when you release the VDKQueue object

- (NSUInteger) numberOfWatchedPaths; // Returns the number of paths that this VDKQueue instance is actively watching.

Expand Down
34 changes: 25 additions & 9 deletions VDKQueue.m
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,6 @@ - (id) init

- (void) dealloc
{
// Shut down the thread that's scanning for kQueue events
_keepWatcherThreadRunning = NO;

// Do this to close all the open file descriptors for files we're watching
[self removeAllPaths];

[_watchedPathEntries release];
_watchedPathEntries = nil;

Expand Down Expand Up @@ -200,6 +194,8 @@ - (VDKQueuePathEntry *) addPathToQueue:(NSString *)path notifyingAbout:(u_int)fl
{
_keepWatcherThreadRunning = YES;
[NSThread detachNewThreadSelector:@selector(watcherThread:) toTarget:self withObject:nil];
// register a custom event that we will trigger to stop the thread
kevent(_coreQueueFD, &(struct kevent){0, EVFILT_USER, EV_ADD, NOTE_FFNOP, 0, NULL}, 1, NULL, 0, &nullts);
}
}

Expand All @@ -218,7 +214,8 @@ - (void) watcherThread:(id)sender
{
int n;
struct kevent ev;
struct timespec timeout = { 1, 0 }; // 1 second timeout. Should be longer, but we need this thread to exit when a kqueue is dealloced, so 1 second timeout is quite a while to wait.
// no timeout needed. Instead of polling every second, we can pass NULL to wait indefinitely
// for vnode events or for an EVFILT_USER event to stop the thread.
int theFD = _coreQueueFD; // So we don't have to risk accessing iVars when the thread is terminated.

NSMutableArray *notesToPost = [[NSMutableArray alloc] initWithCapacity:5];
Expand All @@ -227,11 +224,14 @@ - (void) watcherThread:(id)sender
NSLog(@"watcherThread started.");
#endif

while(_keepWatcherThreadRunning)
while(1)
{
@try
{
n = kevent(theFD, NULL, 0, &ev, 1, &timeout);
n = kevent(theFD, NULL, 0, &ev, 1, NULL);
if (ev.filter == EVFILT_USER) {
break;
}
if (n > 0)
{
//NSLog( @"KEVENT returned %d", n );
Expand Down Expand Up @@ -422,6 +422,22 @@ - (void) removeAllPaths
}


- (void) stopWatching
{
// This method must be called if we want this object to ever be dealloc'd. This is because detachNewThreadSelector:toTarget:withObject:
// retains the target (in this case, self), setting the retainCount to 2. Aside from the memory leak issue, if the thread is never terminated,
// the watcher thread might still try to send messages to a nonexistent delegate, causing your app to crash.
@synchronized(self)
{
// Shut down the thread that's scanning for kQueue events
kevent(_coreQueueFD, &(struct kevent){0, EVFILT_USER, EV_ENABLE, NOTE_TRIGGER, 0, NULL}, 1, NULL, 0, &(struct timespec){0,0});

// Do this to close all the open file descriptors for files we're watching
[_watchedPathEntries removeAllObjects];
}
}


- (NSUInteger) numberOfWatchedPaths
{
NSUInteger count;
Expand Down