From a2e257158f491a0428926d0bf99e9200d3e0617f Mon Sep 17 00:00:00 2001 From: "gtm.daemon" Date: Mon, 22 Mar 2010 18:05:14 +0000 Subject: [Author: dmaclach] Added some KVO debugging help. Specifically forces can access instance variables by KVO to NO in debug mode. R=thomasvl DELTA=340 (269 added, 21 deleted, 50 changed) --- Foundation/GTMNSObject+KeyValueObserving.m | 317 ++++++++++++++++++++++------- 1 file changed, 245 insertions(+), 72 deletions(-) (limited to 'Foundation/GTMNSObject+KeyValueObserving.m') diff --git a/Foundation/GTMNSObject+KeyValueObserving.m b/Foundation/GTMNSObject+KeyValueObserving.m index a665abd..dde77e5 100644 --- a/Foundation/GTMNSObject+KeyValueObserving.m +++ b/Foundation/GTMNSObject+KeyValueObserving.m @@ -1,5 +1,5 @@ // -// GTMNSObject+KeyValueObserving.h +// GTMNSObject+KeyValueObserving.m // // Copyright 2009 Google Inc. // @@ -25,11 +25,12 @@ // This code is based on code by Michael Ash. // See comment in header. - +#import "GTMDefines.h" #import "GTMNSObject+KeyValueObserving.h" #import "GTMDefines.h" #import "GTMDebugSelectorValidation.h" #import "GTMObjC2Runtime.h" +#import "GTMMethodCheck.h" // A singleton that works as a dispatch center for KVO // -[NSObject observeValueForKeyPath:ofObject:change:context:] and turns them @@ -37,14 +38,14 @@ // GTMKeyValueObservingHelpers, and keys them via the key generated by // -dictionaryKeyForObserver:ofObject:forKeyPath:selector. @interface GTMKeyValueObservingCenter : NSObject { -@private + @private NSMutableDictionary *observerHelpers_; } + (id)defaultCenter; - (void)addObserver:(id)observer - ofObject:(id)target + ofObject:(id)target forKeyPath:(NSString *)keyPath selector:(SEL)selector userInfo:(id)userInfo @@ -64,7 +65,7 @@ __weak id observer_; SEL selector_; id userInfo_; - id target_; + __weak id target_; NSString* keyPath_; } @@ -120,7 +121,23 @@ static char* GTMKeyValueObservingHelperContext NSStringFromSelector(selector_)]; } +#if GTM_SUPPORT_GC + +- (void)finalize { + if (target_) { + _GTMDevLog(@"Didn't deregister %@", self); + [self deregister]; + } + [super finalize]; +} + +#endif // GTM_SUPPORT_GC + - (void)dealloc { + if (target_) { + _GTMDevLog(@"Didn't deregister %@", self); + [self deregister]; + } [userInfo_ release]; [keyPath_ release]; [super dealloc]; @@ -152,6 +169,7 @@ static char* GTMKeyValueObservingHelperContext - (void)deregister { [target_ removeObserver:self forKeyPath:keyPath_]; + target_ = nil; } @end @@ -194,8 +212,13 @@ static char* GTMKeyValueObservingHelperContext ofObject:(id)target forKeyPath:(NSString *)keyPath selector:(SEL)selector { - NSString *key = [NSString stringWithFormat:@"%p:%p:%@:%p", - observer, target, keyPath, selector]; + NSString *key = nil; + if (!target && !keyPath && !selector) { + key = [NSString stringWithFormat:@"%p:", observer]; + } else { + key = [NSString stringWithFormat:@"%p:%@:%p:%p", + observer, keyPath, selector, target]; + } return key; } @@ -222,6 +245,7 @@ static char* GTMKeyValueObservingHelperContext if (oldHelper) { _GTMDevLog(@"%@ already observing %@ forKeyPath %@", observer, target, keyPath); + [oldHelper deregister]; } #endif // DEBUG [observerHelpers_ setObject:helper forKey:key]; @@ -237,19 +261,27 @@ static char* GTMKeyValueObservingHelperContext ofObject:target forKeyPath:keyPath selector:selector]; - GTMKeyValueObservingHelper *helper = nil; + NSMutableArray *allValidHelperKeys = [NSMutableArray array]; + NSArray *allValidHelpers = nil; @synchronized(self) { - helper = [[observerHelpers_ objectForKey:key] retain]; + + NSString *helperKey; + GTM_FOREACH_OBJECT(helperKey, [observerHelpers_ allKeys]) { + if ([helperKey hasPrefix:key]) { + [allValidHelperKeys addObject:helperKey]; + } + } #if DEBUG - if (!helper) { + if ([allValidHelperKeys count] == 0) { _GTMDevLog(@"%@ was not observing %@ with keypath %@", observer, target, keyPath); } #endif // DEBUG - [observerHelpers_ removeObjectForKey:key]; + allValidHelpers = [observerHelpers_ objectsForKeys:allValidHelperKeys + notFoundMarker:[NSNull null]]; + [observerHelpers_ removeObjectsForKeys:allValidHelperKeys]; } - [helper deregister]; - [helper release]; + [allValidHelpers makeObjectsPerformSelector:@selector(deregister)]; } @end @@ -265,10 +297,11 @@ static char* GTMKeyValueObservingHelperContext @"Missing observer, keyPath, or selector"); GTMKeyValueObservingCenter *center = [GTMKeyValueObservingCenter defaultCenter]; - GTMAssertSelectorNilOrImplementedWithArguments(observer, - selector, - @encode(GTMKeyValueChangeNotification *), - NULL); + GTMAssertSelectorNilOrImplementedWithArguments( + observer, + selector, + @encode(GTMKeyValueChangeNotification *), + NULL); [center addObserver:observer ofObject:self forKeyPath:keyPath @@ -284,16 +317,26 @@ static char* GTMKeyValueObservingHelperContext @"Missing observer, keyPath, or selector"); GTMKeyValueObservingCenter *center = [GTMKeyValueObservingCenter defaultCenter]; - GTMAssertSelectorNilOrImplementedWithArguments(observer, - selector, - @encode(GTMKeyValueChangeNotification *), - NULL); + GTMAssertSelectorNilOrImplementedWithArguments( + observer, + selector, + @encode(GTMKeyValueChangeNotification *), + NULL); [center removeObserver:observer ofObject:self forKeyPath:keyPath selector:selector]; } - + +- (void)gtm_stopObservingAllKeyPaths { + GTMKeyValueObservingCenter *center + = [GTMKeyValueObservingCenter defaultCenter]; + [center removeObserver:self + ofObject:nil + forKeyPath:nil + selector:Nil]; +} + @end @@ -362,13 +405,82 @@ static char* GTMKeyValueObservingHelperContext #ifdef DEBUG -// This category exists to attempt to help debug tricky KVO issues. -// Set the GTMDebugKVO environment variable to 1 and you will get a whole -// pile of KVO notifications that may help you track down problems. +static void SwizzleMethodsInClass(Class cls, SEL sel1, SEL sel2) { + Method m1 = class_getInstanceMethod(cls, sel1); + Method m2 = class_getInstanceMethod(cls, sel2); + method_exchangeImplementations(m1, m2); +} + +static void SwizzleClassMethodsInClass(Class cls, SEL sel1, SEL sel2) { + Method m1 = class_getClassMethod(cls, sel1); + Method m2 = class_getClassMethod(cls, sel2); + method_exchangeImplementations(m1, m2); +} + +// This category exists to attempt to help deal with tricky KVO issues. +// KVO is a wonderful technology in some ways, but is extremely fragile and +// allows developers a lot of freedom to shoot themselves in the foot. +// Refactoring an app that uses a lot of KVO can be really difficult, as can +// debugging it. +// These are some tools that we have found useful when working with KVO. Note +// that these tools are only on in Debug builds. +// +// We have divided these tools up into two categories: Checks and Debugs. +// +// Debugs +// Debugs are mainly for logging all the KVO/KVC that is occurring in your +// application. To enable our KVO debugging, set the GTMDebugKVO environment +// variable to 1 and you will get a whole pile of KVO logging that may help you +// track down problems. +// bash - export GTMDebugKVO=1 +// csh/tcsh - setenv GTMDebugKVO 1 +// +// Checks +// First we believe that instance variables should be private by default, +// and that any KVO should be done via accessors. Apple by default allows KVO +// to get at instance variables directly. Since our coding standards define +// that instance variables should be @private, we feel that KVO shouldn't be +// breaking this encapsulation. Unfortunately the @private, @protected +// designators are a compile time convention only, and don't get carried over +// into the runtime, so there's no way to check on an individual iVar what +// it's visibility is. We therefore assume that an instance variable is private, +// and disallow KVO access to instance variables. The problem with most KVO +// issues is that they occur at runtime and unless you execute that case you +// may never see the bug until it's too late. We try to force KVO issues to +// rear their head at the time of the observing if at all possible. +// Checks are on by default in debug builds. They can be turned off by defining +// the compile flag GTM_PERFORM_KVO_CHECKS to 0 +// i.e. #define GTM_PERFORM_KVO_CHECKS 0, or set it +// in GCC_PREPROCESSOR_DEFINITIONS. +// +// Checks work at a couple of different levels. +// The most restrictive of the checks is that we set +// |accessInstanceVariablesDirectly| to NO by default. This means that if you +// attempt to perform KVO on an instance variable, you will get an exception +// thrown. +// Also, when adding an observer, we check to see if any member of the path +// starts or ends with _ which by convention denotes an instance variable. If so +// we warn you about attempting to access a ivar directly. + @interface NSObject (GTMDebugKeyValueObserving) @end @implementation NSObject (GTMDebugKeyValueObserving) +GTM_METHOD_CHECK(NSObject, _gtmDebugAddObserver:forKeyPath:options:context:); +GTM_METHOD_CHECK(NSObject, _gtmDebugRemoveObserver:forKeyPath:); +GTM_METHOD_CHECK(NSObject, _gtmDebugWillChangeValueForKey:); +GTM_METHOD_CHECK(NSObject, _gtmDebugDidChangeValueForKey:); +GTM_METHOD_CHECK(NSArray, + _gtmDebugArrayAddObserver:toObjectsAtIndexes:forKeyPath:options:context:); +GTM_METHOD_CHECK(NSArray, + _gtmDebugArrayRemoveObserver:fromObjectsAtIndexes:forKeyPath:); +GTM_METHOD_CHECK(NSObject, + _gtmCheckAddObserver:forKeyPath:options:context:); +GTM_METHOD_CHECK(NSArray, + _gtmCheckAddObserver:toObjectsAtIndexes:forKeyPath:options:context:); +GTM_METHOD_CHECK(NSObject, + _gtmAccessInstanceVariablesDirectly); + + (void)load { NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; NSDictionary *env = [[NSProcessInfo processInfo] environment]; @@ -380,48 +492,43 @@ static char* GTMKeyValueObservingHelperContext debug = ([debugKeyValue hasPrefix:@"Y"] || [debugKeyValue hasPrefix:@"T"] || [debugKeyValue intValue]); } + Class cls = Nil; if (debug) { - Class cls = [NSObject class]; - SEL addSelector - = NSSelectorFromString(@"addObserver:forKeyPath:options:context:"); - SEL removeSelector = NSSelectorFromString(@"removeObserver:forKeyPath:"); - SEL debugAddSelector - = NSSelectorFromString(@"_gtmDebugAddObserver:forKeyPath:options:context:"); - SEL debugRemoveSelector - = NSSelectorFromString(@"_gtmDebugRemoveObserver:forKeyPath:"); - SEL willChangeValueSelector = NSSelectorFromString(@"willChangeValueForKey:"); - SEL didChangeValueSelector = NSSelectorFromString(@"didChangeValueForKey:"); - SEL debugWillChangeValueSelector - = NSSelectorFromString(@"_gtmDebugWillChangeValueForKey:"); - SEL debugDidChangeValueSelector - = NSSelectorFromString(@"_gtmDebugDidChangeValueForKey:"); - - Method m1 = class_getInstanceMethod(cls, addSelector); - Method m2 = class_getInstanceMethod(cls, debugAddSelector); - method_exchangeImplementations(m1, m2); - m1 = class_getInstanceMethod(cls, removeSelector); - m2 = class_getInstanceMethod(cls, debugRemoveSelector); - method_exchangeImplementations(m1, m2); - m1 = class_getInstanceMethod(cls, willChangeValueSelector); - m2 = class_getInstanceMethod(cls, debugWillChangeValueSelector); - method_exchangeImplementations(m1, m2); - m1 = class_getInstanceMethod(cls, didChangeValueSelector); - m2 = class_getInstanceMethod(cls, debugDidChangeValueSelector); - method_exchangeImplementations(m1, m2); - - debugAddSelector - = NSSelectorFromString(@"_gtmDebugArrayAddObserver:forKeyPath:options:context:"); - debugRemoveSelector - = NSSelectorFromString(@"_gtmDebugArrayRemoveObserver:forKeyPath:"); - + cls = [NSObject class]; + SwizzleMethodsInClass(cls, + @selector(addObserver:forKeyPath:options:context:), + @selector(_gtmDebugAddObserver:forKeyPath:options:context:)); + SwizzleMethodsInClass(cls, + @selector(removeObserver:forKeyPath:), + @selector(_gtmDebugRemoveObserver:forKeyPath:)); + SwizzleMethodsInClass(cls, + @selector(willChangeValueForKey:), + @selector(_gtmDebugWillChangeValueForKey:)); + SwizzleMethodsInClass(cls, + @selector(didChangeValueForKey:), + @selector(_gtmDebugDidChangeValueForKey:)); cls = [NSArray class]; - m1 = class_getInstanceMethod(cls, addSelector); - m2 = class_getInstanceMethod(cls, debugAddSelector); - method_exchangeImplementations(m1, m2); - m1 = class_getInstanceMethod(cls, removeSelector); - m2 = class_getInstanceMethod(cls, debugRemoveSelector); - method_exchangeImplementations(m1, m2); + SwizzleMethodsInClass(cls, + @selector(addObserver:toObjectsAtIndexes:forKeyPath:options:context:), + @selector(_gtmDebugArrayAddObserver:toObjectsAtIndexes:forKeyPath:options:context:)); + SwizzleMethodsInClass(cls, + @selector(removeObserver:fromObjectsAtIndexes:forKeyPath:), + @selector(_gtmDebugArrayRemoveObserver:fromObjectsAtIndexes:forKeyPath:)); } +#if GTM_PERFORM_KVO_CHECKS + cls = [NSObject class]; + SwizzleMethodsInClass(cls, + @selector(addObserver:forKeyPath:options:context:), + @selector(_gtmCheckAddObserver:forKeyPath:options:context:)); + SwizzleClassMethodsInClass(cls, + @selector(accessInstanceVariablesDirectly), + @selector(_gtmAccessInstanceVariablesDirectly)); + cls = [NSArray class]; + SwizzleMethodsInClass(cls, + @selector(addObserver:toObjectsAtIndexes:forKeyPath:options:context:), + @selector(_gtmCheckAddObserver:toObjectsAtIndexes:forKeyPath:options:context:)); + +#endif // GTM_PERFORM_KVO_CHECKS [pool release]; } @@ -434,25 +541,34 @@ static char* GTMKeyValueObservingHelperContext options:options context:context]; } -- (void)_gtmDebugArrayAddObserver:(NSObject *)observer +- (void)_gtmDebugArrayAddObserver:(NSObject *)observer + toObjectsAtIndexes:(NSIndexSet *)indexes forKeyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options context:(void *)context { - _GTMDevLog(@"Array adding observer %@ to %@ keypath '%@'", observer, self, keyPath); - [self _gtmDebugArrayAddObserver:observer forKeyPath:keyPath + _GTMDevLog(@"Array adding observer %@ to indexes %@ of %@ keypath '%@'", + observer, indexes, self, keyPath); + [self _gtmDebugArrayAddObserver:observer + toObjectsAtIndexes:indexes + forKeyPath:keyPath options:options context:context]; } -- (void)_gtmDebugRemoveObserver:(NSObject *)observer +- (void)_gtmDebugRemoveObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath { - _GTMDevLog(@"Removing observer %@ from %@ keypath '%@'", observer, self, keyPath); + _GTMDevLog(@"Removing observer %@ from %@ keypath '%@'", + observer, self, keyPath); [self _gtmDebugRemoveObserver:observer forKeyPath:keyPath]; } - (void)_gtmDebugArrayRemoveObserver:(NSObject *)observer + fromObjectsAtIndexes:(NSIndexSet *)indexes forKeyPath:(NSString *)keyPath { - _GTMDevLog(@"Array removing observer %@ from %@ keypath '%@'", observer, self, keyPath); - [self _gtmDebugArrayRemoveObserver:observer forKeyPath:keyPath]; + _GTMDevLog(@"Array removing observer %@ from indexes %@ of %@ keypath '%@'", + indexes, observer, self, keyPath); + [self _gtmDebugArrayRemoveObserver:observer + fromObjectsAtIndexes:indexes + forKeyPath:keyPath]; } - (void)_gtmDebugWillChangeValueForKey:(NSString*)key { @@ -465,7 +581,64 @@ static char* GTMKeyValueObservingHelperContext [self _gtmDebugDidChangeValueForKey:key]; } +#if GTM_PERFORM_KVO_CHECKS + +- (void)_gtmCheckAddObserver:(NSObject *)observer + forKeyPath:(NSString *)keyPath + options:(NSKeyValueObservingOptions)options + context:(void *)context { + NSArray *keyPathElements = [keyPath componentsSeparatedByString:@"."]; + NSString *element; + GTM_FOREACH_OBJECT(element, keyPathElements) { + if ([element hasPrefix:@"_"] || [element hasSuffix:@"_"]) { + _GTMDevLog(@"warning: %@ is registering an observation on what appears " + @"to be a private ivar of %@ (or a sub keyed object) with " + @"element %@ of keyPath %@.", observer, self, element, + keyPath); + } + } + [self _gtmCheckAddObserver:observer + forKeyPath:keyPath + options:options + context:context]; +} + +- (void)_gtmCheckAddObserver:(NSObject *)observer + toObjectsAtIndexes:(NSIndexSet *)indexes + forKeyPath:(NSString *)keyPath + options:(NSKeyValueObservingOptions)options + context:(void *)context { + NSArray *keyPathElements = [keyPath componentsSeparatedByString:@"."]; + NSString *element; + GTM_FOREACH_OBJECT(element, keyPathElements) { + if ([element hasPrefix:@"_"] || [element hasSuffix:@"_"]) { + _GTMDevLog(@"warning: %@ is registering an observation on what appears " + @"to be a private ivar of %@ (or a sub keyed object) with " + @"element %@ of keyPath %@.", observer, self, element, + keyPath); + } + } + [self _gtmCheckAddObserver:observer + toObjectsAtIndexes:indexes + forKeyPath:keyPath + options:options + context:context]; +} + + ++ (BOOL)_gtmAccessInstanceVariablesDirectly { + // Apple has lots of "bad" direct instance variable accesses, so we + // only want to check our code, as opposed to library code. + + // If this turns out to be slow, we may want to consider a cache to speed + // things up. + NSBundle *bundle = [NSBundle bundleForClass:self]; + NSString *path = [bundle bundlePath]; + return [path rangeOfString:@"/Library/"].location != NSNotFound; +} + +#endif // GTM_PERFORM_KVO_CHECKS + @end #endif // DEBUG - -- cgit v1.2.3