From e05fb3871469c7ea70853601a6adc49f82976541 Mon Sep 17 00:00:00 2001 From: Dominic Yu Date: Mon, 25 Dec 2023 01:04:48 -0800 Subject: [PATCH 1/3] fix crash calling nonexistent delegate (also memory leak) --- VDKQueue.h | 2 ++ VDKQueue.m | 22 ++++++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/VDKQueue.h b/VDKQueue.h index 9856a34..fdbe670 100644 --- a/VDKQueue.h +++ b/VDKQueue.h @@ -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.) @@ -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. diff --git a/VDKQueue.m b/VDKQueue.m index f1bc83b..2877216 100644 --- a/VDKQueue.m +++ b/VDKQueue.m @@ -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; @@ -422,6 +416,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 + _keepWatcherThreadRunning = NO; + + // Do this to close all the open file descriptors for files we're watching + [_watchedPathEntries removeAllObjects]; + } +} + + - (NSUInteger) numberOfWatchedPaths { NSUInteger count; From 3f1014f03a2a826c66fb12555581b9350ada2c6f Mon Sep 17 00:00:00 2001 From: Dominic Yu Date: Mon, 25 Dec 2023 01:16:56 -0800 Subject: [PATCH 2/3] don't poll every second for kevents --- VDKQueue.h | 2 +- VDKQueue.m | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/VDKQueue.h b/VDKQueue.h index fdbe670..2abe2a8 100644 --- a/VDKQueue.h +++ b/VDKQueue.h @@ -124,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. } diff --git a/VDKQueue.m b/VDKQueue.m index 2877216..4bb0257 100644 --- a/VDKQueue.m +++ b/VDKQueue.m @@ -194,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); } } @@ -212,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]; @@ -221,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 ); @@ -424,7 +430,7 @@ - (void) stopWatching @synchronized(self) { // Shut down the thread that's scanning for kQueue events - _keepWatcherThreadRunning = NO; + 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]; From f59475fb9a6c488590db294978187a427c017582 Mon Sep 17 00:00:00 2001 From: Dominic Yu Date: Thu, 28 Dec 2023 14:48:23 -0800 Subject: [PATCH 3/3] still need to set _keepWatcherThreadRunning to NO --- VDKQueue.m | 1 + 1 file changed, 1 insertion(+) diff --git a/VDKQueue.m b/VDKQueue.m index 4bb0257..d28f276 100644 --- a/VDKQueue.m +++ b/VDKQueue.m @@ -431,6 +431,7 @@ - (void) stopWatching { // 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}); + _keepWatcherThreadRunning = NO; // Do this to close all the open file descriptors for files we're watching [_watchedPathEntries removeAllObjects];