From cd3e69245dfcdf050856a433d23306e8d39dc629 Mon Sep 17 00:00:00 2001 From: rfm Date: Sun, 17 Nov 2024 16:16:46 +0000 Subject: [PATCH] More leak management fixes --- Headers/GNUstepBase/NSObject+GNUstepBase.h | 44 +++++------- Source/Additions/NSObject+GNUstepBase.m | 84 ++++++++++++++-------- Source/GSLocale.m | 3 +- Source/NSNull.m | 1 - Source/NSObject.m | 12 ++-- Source/NSURLProtocol.m | 33 +++++++-- Tests/base/NSURLHandle/basic.m | 7 +- Tests/base/NSURLProtocol/basic.m | 7 +- Tests/base/NSURLRequest/basic.m | 7 +- 9 files changed, 119 insertions(+), 79 deletions(-) diff --git a/Headers/GNUstepBase/NSObject+GNUstepBase.h b/Headers/GNUstepBase/NSObject+GNUstepBase.h index ec0617358..649b48b2e 100644 --- a/Headers/GNUstepBase/NSObject+GNUstepBase.h +++ b/Headers/GNUstepBase/NSObject+GNUstepBase.h @@ -219,9 +219,9 @@ extern "C" { * set to YES).
* Your class then has two options for performing clean-up when the process * ends: - *

1. Use the +leaked: method to register addresses whose contents are to - * be either ignored or released depending on the clean-up setting in force - * when the program exits. + *

1. Use the +leak: at: method to register addresses whose contents are + * to be either ignored or released depending on the clean-up setting in + * force when the program exits. * This mechanism is simple and should be sufficient for many classes. *

*

2. Implement a +atExit method to be run when the process ends and, @@ -241,36 +241,30 @@ extern "C" { */ + (BOOL) isExiting; -/** This method informs the system that anObject should be retained to - * persist until the process exits. If clean-up is enabled the object - * should be released upon process exit. - * If this method is called while the process is already existing it - * returns nil, otherwise it returnes the retained argument. - * Raises an exception if anObject has already been leaked or if it is - * nil (unless the process is exiting). +/** This method stored anObject at anAddress (retaining it) and notes + * that the object should persist until the process exits. If clean-up + * is enabled the object should be released (and the address content + * zeroed out) upon process exit. + * If this method is called while the process is already exiting it + * simply zeros out the memory location then returns nil, otherwise + * it returns the object stored at the memory location. + * Raises an exception if anObject is nil or anAddress is NULL (unless + * the process is already exiting). + */ ++ (id) leak: (id)anObject at: (id*)anAddress; + +/** DEPRECATED ... use +leak: at: instead. */ + (id) NS_RETURNS_RETAINED leak: (id)anObject; -/** This method informs the system that the object at anAddress has been - * retained to persist until the process exits. If clean-up is enabled - * the object should be released (and the address content zeroed out) - * upon process exit. - * If this method is called while the process is already existing it releases - * the object and zeros out the memory location then returns nil, otherwise - * it returns the object found at the memory location. - * Raises an exception if anAddress (or the object at anAddress) has already - * been leaked or if it is nil (unless the process is exiting). - */ -+ (void) leaked: (id*)anAddress; - -/** DEPRECATED ... use +leaked: instead. +/** DEPRECATED ... use +leak: at: instead. */ + (id) NS_RETURNS_RETAINED leakAt: (id*)anAddress; /** Sets the receiver to have its +atExit method called at the point when * the process terminates.
* Returns YES on success and NO on failure (if the class does not implement - * the method or if it is already registered to call it).
+ * +atExit or if it is already registered to call it).
* Implemented as a call to +registerAtExit: with the selector for the +atExit * method as its argument. */ @@ -279,7 +273,7 @@ extern "C" { /** Sets the receiver to have the specified method called at the point when * the process terminates.
* Returns YES on success and NO on failure (if the class does not implement - * the method ir if it is already registered to call it). + * the method or if it is already registered to call a method at exit). */ + (BOOL) registerAtExit: (SEL)aSelector; diff --git a/Source/Additions/NSObject+GNUstepBase.m b/Source/Additions/NSObject+GNUstepBase.m index 0e3c7b21a..b58aa3666 100644 --- a/Source/Additions/NSObject+GNUstepBase.m +++ b/Source/Additions/NSObject+GNUstepBase.m @@ -180,6 +180,10 @@ handleExit() BOOL unknownThread; isExiting = YES; + /* We turn off zombies during exiting so that we don't leak deallocated + * objects during cleanup. + */ +// NSZombieEnabled = NO; unknownThread = GSRegisterCurrentThread(); ENTER_POOL @@ -193,6 +197,7 @@ handleExit() Method method; IMP msg; +fprintf(stderr, "*** +[%s %s]\n", class_getName(tmp->obj), sel_getName(tmp->sel)); method = class_getClassMethod(tmp->obj, tmp->sel); msg = method_getImplementation(method); if (0 != msg) @@ -200,13 +205,18 @@ handleExit() (*msg)(tmp->obj, tmp->sel); } } - else if (YES == shouldCleanUp) + else if (shouldCleanUp) { if (tmp->at) { - tmp->obj = *(tmp->at); + if (tmp->obj != *(tmp->at)) + { +fprintf(stderr, "*** leaked value %p at %p changed to %p\n", tmp->obj, (const void*)tmp->at, *(tmp->at)); + tmp->obj = *(tmp->at); + } *(tmp->at) = nil; } +fprintf(stderr, "*** -[%s release] %p %p\n", class_getName(object_getClass(tmp->obj)), tmp->obj, (const void*)tmp->at); [tmp->obj release]; } free(tmp); @@ -227,35 +237,22 @@ handleExit() return isExiting; } -+ (id) leakAt: (id*)anAddress -{ - struct exitLink *l; - - l = (struct exitLink*)malloc(sizeof(struct exitLink)); - l->at = anAddress; - l->obj = [*anAddress retain]; - l->sel = 0; - setup(); - [exitLock lock]; - l->next = exited; - exited = l; - [exitLock unlock]; - return l->obj; -} - -+ (void) leaked: (id*)anAddress ++ (id) leak: (id)anObject at: (id*)anAddress { struct exitLink *l; - NSAssert(anAddress != NULL, NSInvalidArgumentException); if (isExiting) { - [*anAddress release]; - *anAddress = nil; + if (anAddress) + { + [*anAddress release]; + *anAddress = nil; + } return nil; } - NSAssert([*anAddress isKindOfClass: [NSObject class]], + NSAssert([anObject isKindOfClass: [NSObject class]], NSInvalidArgumentException); + NSAssert(anAddress != NULL, NSInvalidArgumentException); setup(); [exitLock lock]; for (l = exited; l != NULL; l = l->next) @@ -266,16 +263,17 @@ handleExit() [NSException raise: NSInvalidArgumentException format: @"Repeated use of leak address %p", anAddress]; } - if (*anAddress != nil && *anAddress == l->obj) + if (anObject != nil && anObject == l->obj) { [exitLock unlock]; [NSException raise: NSInvalidArgumentException - format: @"Repeated use of leak object %p", *anAddress]; + format: @"Repeated use of leak object %p", anObject]; } } + ASSIGN(*anAddress, anObject); l = (struct exitLink*)malloc(sizeof(struct exitLink)); l->at = anAddress; - l->obj = *anAddress; + l->obj = anObject; l->sel = 0; l->next = exited; exited = l; @@ -283,6 +281,22 @@ handleExit() return l->obj; } ++ (id) leakAt: (id*)anAddress +{ + struct exitLink *l; + + l = (struct exitLink*)malloc(sizeof(struct exitLink)); + l->at = anAddress; + l->obj = [*anAddress retain]; + l->sel = 0; + setup(); + [exitLock lock]; + l->next = exited; + exited = l; + [exitLock unlock]; + return l->obj; +} + + (id) leak: (id)anObject { struct exitLink *l; @@ -344,10 +358,16 @@ handleExit() [exitLock lock]; for (l = exited; l != 0; l = l->next) { - if (l->obj == self && sel_isEqual(l->sel, sel)) + if (l->obj == self) { - [exitLock unlock]; - return NO; // Already registered + if (sel_isEqual(l->sel, sel)) + { + fprintf(stderr, + "*** +[%s registerAtExit: %s] already registered for %s.\n", + class_getName(self), sel_getName(sel), sel_getName(l->sel)); + [exitLock unlock]; + return NO; // Already registered + } } } l = (struct exitLink*)malloc(sizeof(struct exitLink)); @@ -502,6 +522,12 @@ handleExit() */ @implementation NSObject(GSCleanup) ++ (id) leak: (id)anObject at: (id*)anAddress +{ + ASSIGN(*anAddress, anObject); + return *anAddress; +} + + (id) leakAt: (id*)anAddress { [*anAddress retain]; diff --git a/Source/GSLocale.m b/Source/GSLocale.m index d76b62a56..0868e6f1c 100644 --- a/Source/GSLocale.m +++ b/Source/GSLocale.m @@ -223,8 +223,7 @@ GSDomainFromDefaultLocale(void) */ if (saved == nil) { - saved = [dict copy]; - [NSObject leaked: &saved]; + [NSObject leak: AUTORELEASE([dict copy]) at: &saved]; } /** diff --git a/Source/NSNull.m b/Source/NSNull.m index e31dc6326..bb4b30131 100644 --- a/Source/NSNull.m +++ b/Source/NSNull.m @@ -48,7 +48,6 @@ static NSNull *null = 0; if (null == 0) { null = (NSNull*)NSAllocateObject(self, 0, NSDefaultMallocZone()); - [[NSObject leakAt: &null] release]; } } diff --git a/Source/NSObject.m b/Source/NSObject.m index cd910b712..cfbb57996 100644 --- a/Source/NSObject.m +++ b/Source/NSObject.m @@ -171,13 +171,13 @@ extern void GSLogZombie(id o, SEL sel) } if (c == 0) { - NSLog(@"*** -[??? %@]: message sent to deallocated instance %p", - NSStringFromSelector(sel), o); + fprintf(stderr, "*** -[??? %s]: message sent to deallocated instance %p", + sel_getName(sel), o); } else { - NSLog(@"*** -[%@ %@]: message sent to deallocated instance %p", - c, NSStringFromSelector(sel), o); + fprintf(stderr, "*** -[%s %s]: message sent to deallocated instance %p", + class_getName(c), sel_getName(sel), o); } if (GSPrivateEnvironmentFlag("CRASH_ON_ZOMBIE", NO) == YES) { @@ -814,7 +814,7 @@ NSDeallocateObject(id anObject) (*finalize_imp)(anObject, finalize_sel); AREM(aClass, (id)anObject); - if (NSZombieEnabled == YES) + if (NSZombieEnabled) { #ifdef OBJC_CAP_ARC if (0 != zombieMap) @@ -1086,12 +1086,14 @@ static id gs_weak_load(id obj) + (void) _atExit { +/* NSMapTable *m = nil; GS_MUTEX_LOCK(allocationLock); m = zombieMap; zombieMap = nil; GS_MUTEX_UNLOCK(allocationLock); DESTROY(m); +*/ } /** diff --git a/Source/NSURLProtocol.m b/Source/NSURLProtocol.m index fd1f8f1d7..956a94873 100644 --- a/Source/NSURLProtocol.m +++ b/Source/NSURLProtocol.m @@ -180,6 +180,13 @@ debugWrite(id handle, int len, const unsigned char *ptr) static NSMutableArray *pairCache = nil; static NSLock *pairLock = nil; ++ (void) atExit +{ + [[NSNotificationCenter defaultCenter] removeObserver: self]; + DESTROY(pairLock); + DESTROY(pairCache); +} + + (void) initialize { if (pairCache == nil) @@ -196,6 +203,7 @@ static NSLock *pairLock = nil; [[NSNotificationCenter defaultCenter] addObserver: self selector: @selector(purge:) name: @"GSHousekeeping" object: nil]; + [self registerAtExit]; } } @@ -468,19 +476,36 @@ typedef struct { return o; } ++ (void) atExit +{ + if (placeholder) + { + id o = placeholder; + + placeholder = nil; + NSDeallocateObject(o); + } +fprintf(stderr, "Registered retain count %d\n", (int)[registered retainCount]); + DESTROY(registered); + DESTROY(regLock); +} + + (void) initialize { - if (registered == nil) + static BOOL beenHere = NO; + + if (NO == beenHere) { + beenHere = YES; abstractClass = [NSURLProtocol class]; placeholderClass = [NSURLProtocolPlaceholder class]; + + [self registerAtExit]; + placeholder = (NSURLProtocol*)NSAllocateObject(placeholderClass, 0, NSDefaultMallocZone()); - [[NSObject leakAt: &placeholder] release]; registered = [NSMutableArray new]; - [[NSObject leakAt: ®istered] release]; regLock = [NSLock new]; - [[NSObject leakAt: ®Lock] release]; [self registerClass: [_NSHTTPURLProtocol class]]; [self registerClass: [_NSHTTPSURLProtocol class]]; [self registerClass: [_NSFTPURLProtocol class]]; diff --git a/Tests/base/NSURLHandle/basic.m b/Tests/base/NSURLHandle/basic.m index 2fd8d1574..96e56249d 100644 --- a/Tests/base/NSURLHandle/basic.m +++ b/Tests/base/NSURLHandle/basic.m @@ -18,7 +18,7 @@ int main() httpURL = [NSURL URLWithString: @"http://www.gnustep.org"]; foobarURL = [NSURL URLWithString: @"foobar://localhost/madeupscheme"]; - TEST_FOR_CLASS(@"NSURLHandle", [NSURLHandle alloc], + TEST_FOR_CLASS(@"NSURLHandle", AUTORELEASE([NSURLHandle alloc]), "NSURLHandle +alloc returns an NSURLHandle"); PASS_EXCEPTION([DummyHandle cachedHandleForURL: httpURL];, @@ -29,14 +29,11 @@ int main() PASS([cls canInitWithURL: httpURL] == YES, "Appropriate subclass found for +URLHandleClassForURL:"); - handle1 = [[cls alloc] initWithURL: httpURL cached: YES]; + handle1 = AUTORELEASE([[cls alloc] initWithURL: httpURL cached: YES]); handle2 = [NSURLHandle cachedHandleForURL: httpURL]; PASS(handle2 != nil, "Available handle returned from cache"); - [handle1 autorelease]; - [cls autorelease]; - #if !defined(GNUSTEP_BASE_LIBRARY) PASS(NO, "URLHandleClassForURL: seems to hang on MacOS-X when given an unknown URL scheme ... you may want to check to see if it has been fixed"); #else diff --git a/Tests/base/NSURLProtocol/basic.m b/Tests/base/NSURLProtocol/basic.m index 5ebd6f6c4..3afd6f05e 100644 --- a/Tests/base/NSURLProtocol/basic.m +++ b/Tests/base/NSURLProtocol/basic.m @@ -11,10 +11,10 @@ int main() httpURL = [NSURL URLWithString: @"http://www.gnustep.org"]; - TEST_FOR_CLASS(@"NSURLProtocol", [NSURLProtocol alloc], + TEST_FOR_CLASS(@"NSURLProtocol", AUTORELEASE([NSURLProtocol alloc]), "NSURLProtocol +alloc returns an NSURLProtocol"); - mutable = [[NSMutableURLRequest requestWithURL: httpURL] retain]; + mutable = [NSMutableURLRequest requestWithURL: httpURL]; PASS_EXCEPTION([NSURLProtocol canInitWithRequest: mutable], nil, "NSURLProtocol +canInitWithRequest throws an exeception (subclasses should be used)"); @@ -22,13 +22,12 @@ int main() TEST_FOR_CLASS(@"NSURLRequest", canon, "NSURLProtocol +canonicalRequestForRequest: returns an NSURLProtocol"); - copy = [mutable copy]; + copy = AUTORELEASE([mutable copy]); PASS([NSURLProtocol requestIsCacheEquivalent: mutable toRequest: copy], "NSURLProtocol +requestIsCacheEquivalent:toRequest returns YES with a request and its copy"); [copy setHTTPMethod: @"POST"]; PASS([NSURLProtocol requestIsCacheEquivalent: mutable toRequest: copy] == NO, "NSURLProtocol +requestIsCacheEquivalent:toRequest returns NO after a method change"); - [copy release]; [arp release]; arp = nil; return 0; diff --git a/Tests/base/NSURLRequest/basic.m b/Tests/base/NSURLRequest/basic.m index 01c1e84f5..11efa966d 100644 --- a/Tests/base/NSURLRequest/basic.m +++ b/Tests/base/NSURLRequest/basic.m @@ -13,7 +13,7 @@ int main() httpURL = [NSURL URLWithString: @"http://www.gnustep.org"]; foobarURL = [NSURL URLWithString: @"foobar://localhost/madeupscheme"]; - TEST_FOR_CLASS(@"NSURLRequest", [NSURLRequest alloc], + TEST_FOR_CLASS(@"NSURLRequest", AUTORELEASE([NSURLRequest alloc]), "NSURLRequest +alloc returns an NSURLRequest"); request = [NSURLRequest requestWithURL: httpURL]; @@ -28,7 +28,7 @@ int main() PASS(request != nil, "NSURLRequest +requestWithURL returns a request from an invalid URL (unknown scheme)"); - mutable = [request mutableCopy]; + mutable = AUTORELEASE([request mutableCopy]); PASS(mutable != nil && [mutable isKindOfClass:[NSMutableURLRequest class]], "NSURLRequest -mutableCopy returns a mutable request"); [mutable setHTTPMethod: @"POST"]; @@ -53,9 +53,8 @@ int main() [mutable setValue: nil forHTTPHeaderField: @"gnustep"]; expected = [NSDictionary dictionaryWithObjectsAndKeys:@"object", @"key", nil]; PASS_EQUAL([mutable allHTTPHeaderFields], expected, "Remove header field"); - [mutable release]; - mutable = [NSMutableURLRequest new]; + mutable = AUTORELEASE([NSMutableURLRequest new]); PASS(mutable != nil && [mutable isKindOfClass:[NSMutableURLRequest class]], "NSURLRequest +new returns a mutable request");