Permalink
Browse files

Implement reference counting inside IMBFSEventsWatcher, taking care t…

…o remove FSEvents registrations only after the number of removePath calls balances the number of addPath calls that we have received.
  • Loading branch information...
1 parent 717c935 commit 950f6e272a0549727022750362391252a0896a88 @danielpunkass committed Apr 13, 2012
Showing with 384 additions and 25 deletions.
  1. +3 −0 UKFSEventsWatcher.h
  2. +83 −25 UKFSEventsWatcher.m
  3. +281 −0 UKFSEventsWatcher.m.orig
  4. +17 −0 UKFSEventsWatcher.m.rej
View
@@ -31,6 +31,9 @@
CFTimeInterval latency; // Time that must pass before events are being sent.
FSEventStreamCreateFlags flags; // See FSEvents.h
NSMutableDictionary* eventStreams; // List of FSEventStreamRef pointers in NSValues, with the pathnames as their keys.
+ NSMutableDictionary* pathReferenceCounts;// To support a client adding the same path multiple times, we
+ // count the number of times it's been added, and only remove when
+ // the number goes to zero...
}
+ (id) sharedFileWatcher;
View
@@ -90,6 +90,7 @@ -(id) init
latency = 1.0;
flags = kFSEventStreamCreateFlagUseCFTypes | kFSEventStreamCreateFlagWatchRoot;
eventStreams = [[NSMutableDictionary alloc] init];
+ pathReferenceCounts = [[NSMutableDictionary alloc] init];
}
return self;
@@ -103,6 +104,7 @@ -(void) dealloc
{
[self removeAllPaths];
[eventStreams release];
+ [pathReferenceCounts release];
[super dealloc];
}
@@ -199,31 +201,54 @@ - (NSString*) pathToParentFolderOfFile:(NSString*)inPath
-(void) addPath: (NSString*)path
{
+ BOOL succeeded = YES;
path = [self pathToParentFolderOfFile:path];
NSArray* paths = [NSArray arrayWithObject:path];
-
- FSEventStreamContext context;
- context.version = 0;
- context.info = (void*) self;
- context.retain = NULL;
- context.release = NULL;
- context.copyDescription = NULL;
-
- FSEventStreamRef stream = FSEventStreamCreate(NULL,&FSEventCallback,&context,(CFArrayRef)paths,kFSEventStreamEventIdSinceNow,latency,flags);
- if (stream)
+ NSUInteger currentRegistrationCount = 0;
+
+ // Do we already have a stream scheduled for this path?
+ // NOTE: Synchronize the whole thing so we don't run the risk of the current count changing while
+ // we're busy updating it with our new addition.
+ @synchronized (self)
{
- FSEventStreamScheduleWithRunLoop(stream,CFRunLoopGetMain(),kCFRunLoopCommonModes);
- FSEventStreamStart(stream);
+ NSNumber* currentRegistrationCountNumber = [pathReferenceCounts objectForKey:path];
+ if (currentRegistrationCountNumber != nil)
+ {
+ currentRegistrationCount = [currentRegistrationCountNumber unsignedIntValue];
+ }
+
+ if (currentRegistrationCount == 0)
+ {
+ FSEventStreamContext context;
+ context.version = 0;
+ context.info = (void*) self;
+ context.retain = NULL;
+ context.release = NULL;
+ context.copyDescription = NULL;
+
+ FSEventStreamRef stream = FSEventStreamCreate(NULL,&FSEventCallback,&context,(CFArrayRef)paths,kFSEventStreamEventIdSinceNow,latency,flags);
+
+ if (stream)
+ {
+ FSEventStreamScheduleWithRunLoop(stream,CFRunLoopGetMain(),kCFRunLoopCommonModes);
+ FSEventStreamStart(stream);
- @synchronized (self)
+ [eventStreams setObject:[NSValue valueWithPointer:stream] forKey:path];
+ }
+ else
+ {
+ NSLog( @"UKFSEventsWatcher addPath:%@ failed",path);
+ succeeded = NO;
+ }
+ }
+
+ if (succeeded)
{
- [eventStreams setObject:[NSValue valueWithPointer:stream] forKey:path];
- }
- }
- else
- {
- NSLog( @"UKFSEventsWatcher addPath:%@ failed",path);
+ currentRegistrationCount = currentRegistrationCount + 1;
+ NSNumber* newCountNumber = [NSNumber numberWithUnsignedInt:currentRegistrationCount];
+ [pathReferenceCounts setObject:newCountNumber forKey:path];
+ }
}
}
@@ -238,17 +263,41 @@ -(void) removePath: (NSString*)path
// the normalization done in addPath to make sure removePath for the same path will succeed.
path = [self pathToParentFolderOfFile:path];
- NSValue* value = nil;
-
+ NSValue* valueToRemove = nil;
+
@synchronized (self)
{
- value = [[[eventStreams objectForKey:path] retain] autorelease];
- [eventStreams removeObjectForKey:path];
+ NSUInteger currentRegistrationCount = 0;
+ NSNumber* currentRegistrationCountNumber = [pathReferenceCounts objectForKey:path];
+ if (currentRegistrationCountNumber != nil)
+ {
+ currentRegistrationCount = [currentRegistrationCountNumber unsignedIntValue];
+ }
+
+ // We are sometimes asked to removePath on a path that we were never asked to add. That's
+ // OK - it just means they are being extra-certain before (probably) adding it for the
+ // first time...
+ if (currentRegistrationCount > 0)
+ {
+ NSUInteger newRegistrationCount = currentRegistrationCount - 1;
+
+ // Clear everything out if we've gone to zero, otherwise just update with te new count
+ if (newRegistrationCount == 0)
+ {
+ valueToRemove = [[[eventStreams objectForKey:path] retain] autorelease];
+ [eventStreams removeObjectForKey:path];
+ [pathReferenceCounts removeObjectForKey:path];
+ }
+ else
+ {
+ [pathReferenceCounts setObject:[NSNumber numberWithUnsignedInt:newRegistrationCount] forKey:path];
+ }
+ }
}
- if (value)
+ if (valueToRemove)
{
- FSEventStreamRef stream = [value pointerValue];
+ FSEventStreamRef stream = [valueToRemove pointerValue];
if (stream)
{
@@ -271,6 +320,15 @@ -(void) removeAllPaths
while (path = [paths nextObject])
{
+ // Kind of a hack, but to get the remove to work as expected, we need
+ // to make sure the reference count shows up as 1. The client in this
+ // case is asking us to disregard all reference counts and just remove
+ // everything ...
+ @synchronized (self)
+ {
+ [pathReferenceCounts setObject:[NSNumber numberWithUnsignedInt:1] forKey:path];
+ }
+
[self removePath:path];
}
}
Oops, something went wrong.

1 comment on commit 950f6e2

Major apologies that it's taken me so long to get round to looking into this Daniel. I've not had experience with the FSEvents system myself so trusting your diagnosis to be right :)

I do have a request though: implementing our own reference counting scheme seems a little bit clunky at the moment. What if we use an NSCountedSet to store the paths, instead of the mutable dictionary you've added here?

Please sign in to comment.