Skip to content

Move GTMNSObject+KeyValueObserving over to ARC #548

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

Merged
dmaclach merged 1 commit into from
Aug 22, 2025
Merged

Move GTMNSObject+KeyValueObserving over to ARC #548

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
2 changes: 1 addition & 1 deletion Foundation/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ objc_library(
"GTMNSObject+KeyValueObserving.h",
],
copts = ["-IDebugUtils"],
non_arc_srcs = [
srcs = [
"GTMNSObject+KeyValueObserving.m",
],
sdk_frameworks = [
Expand Down
129 changes: 52 additions & 77 deletions Foundation/GTMNSObject+KeyValueObserving.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@
// See comment in header.
#import "GTMNSObject+KeyValueObserving.h"

#import <libkern/OSAtomic.h>
#include <objc/runtime.h>
#import <stdatomic.h>

#import "GTMDefines.h"
#import "GTMDebugSelectorValidation.h"
Expand Down Expand Up @@ -64,11 +62,10 @@ - (id)dictionaryKeyForObserver:(id)observer
@end

@interface GTMKeyValueObservingHelper : NSObject {
@private

@thomasvl thomasvl Aug 20, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this might have been needed?

@dmaclach dmaclach Aug 21, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think this was needed?

@thomasvl thomasvl Aug 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake, I was looking at the errors for the CI jobs failing, and didn't read close enough; I was thinking it was something about the decl, but the problem is https://github.com/google/google-toolbox-for-mac/blob/main/GoogleToolboxForMac.podspec#L69-L74

Probably needs to have inserted:

    sp.requires_arc = 'Foundation/GTMNSObject+KeyValueObserving.{h,m}'

So it's like the Logger entry: https://github.com/google/google-toolbox-for-mac/blob/main/GoogleToolboxForMac.podspec#L76-L78

GTM_WEAK id observer_;
__weak id observer_;
SEL selector_;
id userInfo_;
GTM_WEAK id target_;
__weak id target_;
NSString* keyPath_;
}

Expand Down Expand Up @@ -106,10 +103,10 @@ - (instancetype)initWithObserver:(id)observer
if((self = [super init])) {
observer_ = observer;
selector_ = selector;
userInfo_ = [userInfo retain];
userInfo_ = userInfo;

target_ = target;
keyPath_ = [keyPath retain];
keyPath_ = keyPath;

[target addObserver:self
forKeyPath:keyPath
Expand All @@ -131,9 +128,6 @@ - (void)dealloc {
_GTMDevLog(@"Didn't deregister %@", self);
[self deregister];
}
[userInfo_ release];
[keyPath_ release];
[super dealloc];
}

- (void)observeValueForKeyPath:(NSString *)keyPath
Expand All @@ -146,8 +140,11 @@ - (void)observeValueForKeyPath:(NSString *)keyPath
ofObject:object
userInfo:userInfo_
change:change];
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Warc-performSelector-leaks"
// We are good here because the selector should not return anything.
[observer_ performSelector:selector_ withObject:notification];
[notification release];
#pragma clang diagnostic pop
} else {
// COV_NF_START
// There's no way this should ever be called.
Expand All @@ -163,26 +160,20 @@ - (void)observeValueForKeyPath:(NSString *)keyPath
- (void)deregister {
[target_ removeObserver:self forKeyPath:keyPath_];
target_ = nil;
observer_ = nil;
}

@end

@implementation GTMKeyValueObservingCenter

+ (instancetype)defaultCenter {
static _Atomic (GTMKeyValueObservingCenter *)center = nil;
if(!center) {
// do a bit of clever atomic setting to make this thread safe
// if two threads try to set simultaneously, one will fail
// and the other will set things up so that the failing thread
// gets the shared center
GTMKeyValueObservingCenter *newCenter = [[self alloc] init];
GTMKeyValueObservingCenter *expected = nil;
if (!atomic_compare_exchange_strong(&center, &expected, newCenter)) {
[newCenter release]; // COV_NF_LINE no guarantee we'll hit this line
}
}
return center;
static dispatch_once_t onceToken;
static GTMKeyValueObservingCenter *center;
dispatch_once(&onceToken, ^{
center = [[self alloc] init];
});
return center;
}

- (instancetype)init {
Expand All @@ -192,14 +183,6 @@ - (instancetype)init {
return self;
}

// COV_NF_START
// Singletons don't get deallocated
- (void)dealloc {
[observerHelpers_ release];
[super dealloc];
}
// COV_NF_END

- (id)dictionaryKeyForObserver:(id)observer
ofObject:(id)target
forKeyPath:(NSString *)keyPath
Expand Down Expand Up @@ -240,7 +223,6 @@ - (void)addObserver:(id)observer
}
[observerHelpers_ setObject:helper forKey:key];
}
[helper release];
}

- (void)removeObserver:(id)observer
Expand Down Expand Up @@ -338,21 +320,13 @@ - (instancetype)initWithKeyPath:(NSString *)keyPath
change:(NSDictionary *)change {
if ((self = [super init])) {
keyPath_ = [keyPath copy];
object_ = [object retain];
userInfo_ = [userInfo retain];
change_ = [change retain];
object_ = object;
userInfo_ = userInfo;
change_ = change;
}
return self;
}

- (void)dealloc {
[keyPath_ release];
[object_ release];
[userInfo_ release];
[change_ release];
[super dealloc];
}

- (instancetype)copyWithZone:(NSZone *)zone {
return [[[self class] allocWithZone:zone] initWithKeyPath:keyPath_
ofObject:object_
Expand Down Expand Up @@ -448,40 +422,41 @@ @implementation NSObject (GTMDebugKeyValueObserving)
_gtmDebugArrayRemoveObserver:fromObjectsAtIndexes:forKeyPath:);

+ (void)load {
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
NSDictionary *env = [[NSProcessInfo processInfo] environment];
id debugKeyValue = [env valueForKey:@"GTMDebugKVO"];
BOOL debug = NO;
if ([debugKeyValue isKindOfClass:[NSNumber class]]) {
debug = [debugKeyValue intValue] != 0 ? YES : NO;
} else if ([debugKeyValue isKindOfClass:[NSString class]]) {
debug = ([debugKeyValue hasPrefix:@"Y"] || [debugKeyValue hasPrefix:@"T"] ||
[debugKeyValue intValue]);
}
Class cls = Nil;
if (debug) {
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];
SwizzleMethodsInClass(cls,
@selector(addObserver:toObjectsAtIndexes:forKeyPath:options:context:),
@selector(_gtmDebugArrayAddObserver:toObjectsAtIndexes:forKeyPath:options:context:));
SwizzleMethodsInClass(cls,
@selector(removeObserver:fromObjectsAtIndexes:forKeyPath:),
@selector(_gtmDebugArrayRemoveObserver:fromObjectsAtIndexes:forKeyPath:));
@autoreleasepool {

NSDictionary *env = [[NSProcessInfo processInfo] environment];
id debugKeyValue = [env valueForKey:@"GTMDebugKVO"];
BOOL debug = NO;
if ([debugKeyValue isKindOfClass:[NSNumber class]]) {
debug = [debugKeyValue intValue] != 0 ? YES : NO;
} else if ([debugKeyValue isKindOfClass:[NSString class]]) {
debug = ([debugKeyValue hasPrefix:@"Y"] || [debugKeyValue hasPrefix:@"T"] ||
[debugKeyValue intValue]);
}
Class cls = Nil;
if (debug) {
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];
SwizzleMethodsInClass(cls,
@selector(addObserver:toObjectsAtIndexes:forKeyPath:options:context:),
@selector(_gtmDebugArrayAddObserver:toObjectsAtIndexes:forKeyPath:options:context:));
SwizzleMethodsInClass(cls,
@selector(removeObserver:fromObjectsAtIndexes:forKeyPath:),
@selector(_gtmDebugArrayRemoveObserver:fromObjectsAtIndexes:forKeyPath:));
}
}
[pool drain];
}

- (void)_gtmDebugAddObserver:(NSObject *)observer
Expand Down
2 changes: 1 addition & 1 deletion Foundation/GTMNSObject+KeyValueObservingTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
@interface GTMNSObject_KeyValueObservingTest : GTMTestCase {
int32_t count_;
NSMutableDictionary *dict_;
GTM_WEAK NSString *expectedValue_;
NSString *expectedValue_;
}

- (void)observeValueChange:(GTMKeyValueChangeNotification *)notification;
Expand Down
2 changes: 1 addition & 1 deletion GTM.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
8B5769AB21CD7ACF00D924D3 /* GTMTimeUtilsTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B5769A921CD798000D924D3 /* GTMTimeUtilsTest.m */; };
8B61FDC00E4CDB8000FF9C21 /* GTMStackTrace.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B61FDBF0E4CDB8000FF9C21 /* GTMStackTrace.m */; };
8B6C15930F356E6400E51E5D /* GTMNSObject+KeyValueObserving.h in Headers */ = {isa = PBXBuildFile; fileRef = 8B6C15910F356E6400E51E5D /* GTMNSObject+KeyValueObserving.h */; settings = {ATTRIBUTES = (Public, ); }; };
8B6C15940F356E6400E51E5D /* GTMNSObject+KeyValueObserving.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B6C15920F356E6400E51E5D /* GTMNSObject+KeyValueObserving.m */; };
8B6C15940F356E6400E51E5D /* GTMNSObject+KeyValueObserving.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B6C15920F356E6400E51E5D /* GTMNSObject+KeyValueObserving.m */; settings = {COMPILER_FLAGS = "-fobjc-arc"; }; };
8B6F4B630E8856CA00425D9F /* GTMDebugThreadValidation.h in Headers */ = {isa = PBXBuildFile; fileRef = 8B6F4B610E8856CA00425D9F /* GTMDebugThreadValidation.h */; settings = {ATTRIBUTES = (Public, ); }; };
8B6F4B640E8856CA00425D9F /* GTMDebugThreadValidation.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B6F4B620E8856CA00425D9F /* GTMDebugThreadValidation.m */; };
8B7DCBA50DFF0EFF0017E983 /* GoogleToolboxForMac.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = F42E086D0D199A5B00D5DDE0 /* GoogleToolboxForMac.framework */; };
Expand Down
21 changes: 20 additions & 1 deletion GTMiPhone.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
8B82CF081D9C1C3B007182AA /* GTMNSData+zlib.m in Sources */ = {isa = PBXBuildFile; fileRef = 8BC0477F0DAE928A00C2D1CA /* GTMNSData+zlib.m */; };
8B82CF0A1D9C1C3B007182AA /* GTMNSFileManager+Path.m in Sources */ = {isa = PBXBuildFile; fileRef = 8BC047850DAE928A00C2D1CA /* GTMNSFileManager+Path.m */; };
8B82CF0B1D9C1C3B007182AA /* GTMNSFileHandle+UniqueName.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B2908B011F8E7070064F50F /* GTMNSFileHandle+UniqueName.m */; };
8B82CF0D1D9C1C3B007182AA /* GTMNSObject+KeyValueObserving.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B6C18720F3769D200E51E5D /* GTMNSObject+KeyValueObserving.m */; };
8B82CF0D1D9C1C3B007182AA /* GTMNSObject+KeyValueObserving.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B6C18720F3769D200E51E5D /* GTMNSObject+KeyValueObserving.m */; settings = {COMPILER_FLAGS = "-fobjc-arc"; }; };
8B82CF0F1D9C1C3B007182AA /* GTMNSString+HTML.m in Sources */ = {isa = PBXBuildFile; fileRef = 8BC047880DAE928A00C2D1CA /* GTMNSString+HTML.m */; };
8B82CF111D9C1C3B007182AA /* GTMNSString+XML.m in Sources */ = {isa = PBXBuildFile; fileRef = 8BC0478B0DAE928A00C2D1CA /* GTMNSString+XML.m */; };
8B82CF121D9C1C3B007182AA /* GTMNSThread+Blocks.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B6FF392151A664600B0642B /* GTMNSThread+Blocks.m */; };
Expand Down Expand Up @@ -64,6 +64,16 @@
F4A1EEA52B62D38F008B2FC1 /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = F4A1EEA12B62D338008B2FC1 /* PrivacyInfo.xcprivacy */; };
/* End PBXBuildFile section */

/* Begin PBXContainerItemProxy section */
8BFA30492E562C740037F469 /* PBXContainerItemProxy */ = {
isa = PBXContainerItemProxy;
containerPortal = 29B97313FDCFA39411CA2CEA /* Project object */;
proxyType = 1;
remoteGlobalIDString = 8B82CEF51D9C17DE007182AA;
remoteInfo = GTM;
};
/* End PBXContainerItemProxy section */

/* Begin PBXCopyFilesBuildPhase section */
8B82CEF41D9C17DE007182AA /* CopyFiles */ = {
isa = PBXCopyFilesBuildPhase;
Expand Down Expand Up @@ -462,6 +472,7 @@
buildRules = (
);
dependencies = (
8BFA304A2E562C740037F469 /* PBXTargetDependency */,
);
name = GTMTests;
productName = GTMTests;
Expand Down Expand Up @@ -593,6 +604,14 @@
};
/* End PBXSourcesBuildPhase section */

/* Begin PBXTargetDependency section */
8BFA304A2E562C740037F469 /* PBXTargetDependency */ = {
isa = PBXTargetDependency;
target = 8B82CEF51D9C17DE007182AA /* GTM */;
targetProxy = 8BFA30492E562C740037F469 /* PBXContainerItemProxy */;
};
/* End PBXTargetDependency section */

/* Begin XCBuildConfiguration section */
8B82CEFD1D9C17DE007182AA /* Debug */ = {
isa = XCBuildConfiguration;
Expand Down
2 changes: 2 additions & 0 deletions GoogleToolboxForMac.podspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ Pod::Spec.new do |s|
s.subspec 'KVO' do |sp|
sp.source_files =
'Foundation/GTMNSObject+KeyValueObserving.{h,m}'
sp.requires_arc =
'Foundation/GTMNSObject+KeyValueObserving.{h,m}'
sp.dependency 'GoogleToolboxForMac/Core', "#{s.version}"
sp.dependency 'GoogleToolboxForMac/DebugUtils', "#{s.version}"
end
Expand Down