From 2278c79f62fa0fdf632807adec97bf7ba5fb5972 Mon Sep 17 00:00:00 2001 From: rfm Date: Wed, 20 Nov 2024 14:27:44 +0000 Subject: [PATCH 01/16] Add +trackOwnership method to trace tetain/release for objects of a class --- Headers/GNUstepBase/NSObject+GNUstepBase.h | 2 + Source/Additions/NSObject+GNUstepBase.m | 107 +++++++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/Headers/GNUstepBase/NSObject+GNUstepBase.h b/Headers/GNUstepBase/NSObject+GNUstepBase.h index 166c13a10..fdaee5240 100644 --- a/Headers/GNUstepBase/NSObject+GNUstepBase.h +++ b/Headers/GNUstepBase/NSObject+GNUstepBase.h @@ -295,6 +295,8 @@ extern "C" { */ + (BOOL) shouldCleanUp; ++ (void) trackOwnership; + @end /* Macro to take an autoreleased object and either make it immutable or diff --git a/Source/Additions/NSObject+GNUstepBase.m b/Source/Additions/NSObject+GNUstepBase.m index 316b6edbc..8adcdd28e 100644 --- a/Source/Additions/NSObject+GNUstepBase.m +++ b/Source/Additions/NSObject+GNUstepBase.m @@ -419,6 +419,113 @@ handleExit() return shouldCleanUp; } + + +struct trackLink { + struct trackLink *next; + Class cls; // Class whose instaances are tracked. + IMP release; // Original -release implementation + IMP retain; // Original -retain implementation +}; + +static struct trackLink *tracked = 0; +static gs_mutex_t trackLock = GS_MUTEX_INIT_STATIC; + +- (void) _replacementRelease +{ + struct trackLink *l; + Class c = object_getClass(self); + IMP release = 0; + unsigned rc; + + GS_MUTEX_LOCK(trackLock); + l = tracked; + while (l) + { + if (l->cls == c) + { + release = l->release; + break; + } + l = l->next; + } + GS_MUTEX_UNLOCK(trackLock); + rc = (unsigned)[self retainCount]; + NSLog(@"-[%p release] %u->%u at %@", + self, rc, rc-1, [NSThread callStackSymbols]); + (*release)(self, _cmd); +} +- (id) _replacementRetain +{ + struct trackLink *l; + Class c = object_getClass(self); + IMP retain = 0; + id result; + unsigned rc; + + GS_MUTEX_LOCK(trackLock); + l = tracked; + while (l) + { + if (l->cls == c) + { + retain = l->retain; + break; + } + l = l->next; + } + GS_MUTEX_UNLOCK(trackLock); + rc = (unsigned)[self retainCount]; + result = (*retain)(self, _cmd); + NSLog(@"-[%p retain] %u->%u at %@", + self, rc, (unsigned)[self retainCount], [NSThread callStackSymbols]); + return result; +} + ++ (void) trackOwnership +{ + Method replacementRelease; + Method replacementRetain; + Class c = object_getClass(self); + struct trackLink *l; + + if (class_isMetaClass(c)) + { + c = self; + } + + GS_MUTEX_LOCK(trackLock); + l = tracked; + while (l) + { + if (l->cls == c) + { + GS_MUTEX_UNLOCK(trackLock); + return; + } + l = l->next; + } + + replacementRelease = class_getInstanceMethod([NSObject class], + @selector(_replacementRelease)); + replacementRetain = class_getInstanceMethod([NSObject class], + @selector(_replacementRetain)); + + l = (struct trackLink*)malloc(sizeof(struct trackLink)); + l->cls = c; + l->release = class_getMethodImplementation(c, @selector(release)); + class_replaceMethod(c, @selector(release), + method_getImplementation(replacementRelease), + method_getTypeEncoding(replacementRelease)); + l->retain = class_getMethodImplementation(c, @selector(retain)); + class_replaceMethod(c, @selector(retain), + method_getImplementation(replacementRetain), + method_getTypeEncoding(replacementRetain)); + l->next = tracked; + tracked = l; + GS_MUTEX_UNLOCK(trackLock); +} + @end #else From fff13fc7ac9bcec8cad6a065f4e6b7c24ecf0138 Mon Sep 17 00:00:00 2001 From: rfm Date: Wed, 20 Nov 2024 14:28:46 +0000 Subject: [PATCH 02/16] GSInlineArray seems to cause false positives with LeakSanitizer, so avoid it --- Source/GSArray.m | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/Source/GSArray.m b/Source/GSArray.m index 251ca3131..9cb68147d 100644 --- a/Source/GSArray.m +++ b/Source/GSArray.m @@ -40,6 +40,7 @@ static SEL eqSel; static SEL oaiSel; +static Class GSArrayClass; static Class GSInlineArrayClass; /* This class stores objects inline in data beyond the end of the instance. */ @@ -107,6 +108,7 @@ static Class GSInlineArrayClass; [self setVersion: 1]; eqSel = @selector(isEqual:); oaiSel = @selector(objectAtIndex:); + GSArrayClass = self; GSInlineArrayClass = [GSInlineArray class]; } } @@ -1195,8 +1197,17 @@ static Class GSInlineArrayClass; - (id) initWithObjects: (const id[])objects count: (NSUInteger)count { - self = (id)NSAllocateObject(GSInlineArrayClass, sizeof(id)*count, - [self zone]); + NSZone *z = [self zone]; + + /* The gnustep-make -asan=yes option enables LeakSanitizer but (Nov2024) + * that produces false positives for items held in an inline array, so + * we use the less efficient class in that case. + */ +#if defined(GS_WITH_ASAN) + self = (id)NSAllocateObject(GSArrayClass, 0, z); +#else + self = (id)NSAllocateObject(GSInlineArrayClass, sizeof(id)*count, z); +#endif return [self initWithObjects: objects count: count]; } From 92ef562ebefaca69f6ab4d300d05c05679ac2187 Mon Sep 17 00:00:00 2001 From: rfm Date: Wed, 20 Nov 2024 14:29:41 +0000 Subject: [PATCH 03/16] Fix a few leaks --- Tools/AGSParser.m | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/AGSParser.m b/Tools/AGSParser.m index 79dc0efde..dce64cbb2 100644 --- a/Tools/AGSParser.m +++ b/Tools/AGSParser.m @@ -320,6 +320,7 @@ equalTypes(NSArray *t1, NSArray *t2) - (void) dealloc { + [self reset]; DESTROY(wordMap); DESTROY(ifStack); DESTROY(declared); @@ -1680,7 +1681,7 @@ recheck: /* We want to be able to parse new comments while retaining the originally parsed comment for the enum/union/struct. */ - introComment = [comment copy]; + introComment = AUTORELEASE([comment copy]); DESTROY(comment); pos++; /* Skip '{' */ From 9189f1bca67a33c2284a499ba16729cf7d4bd3a1 Mon Sep 17 00:00:00 2001 From: rfm Date: Wed, 20 Nov 2024 15:27:22 +0000 Subject: [PATCH 04/16] Fix another leak --- Tools/AGSParser.m | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/Tools/AGSParser.m b/Tools/AGSParser.m index dce64cbb2..f8458e68a 100644 --- a/Tools/AGSParser.m +++ b/Tools/AGSParser.m @@ -1486,8 +1486,8 @@ recheck: - (NSMutableArray*) parseDeclarations { + IF_NO_ARC(NSAutoreleasePool *arp = [NSAutoreleasePool new];) NSMutableArray *declarations = [NSMutableArray array]; - CREATE_AUTORELEASE_POOL(arp); static NSSet *qualifiers = nil; static NSSet *keep = nil; NSString *baseName = nil; @@ -1516,7 +1516,7 @@ recheck: @"unsigned", @"volatile", nil]; - IF_NO_ARC([qualifiers retain];) + IF_NO_ARC(qualifiers = [qualifiers retain];) keep = [NSSet setWithObjects: @"const", @"long", @@ -1525,7 +1525,7 @@ recheck: @"unsigned", @"volatile", nil]; - IF_NO_ARC([keep retain];) + IF_NO_ARC(keep = [keep retain];) } { @@ -1573,7 +1573,7 @@ recheck: pos++; [self skipSpaces]; } - IF_NO_ARC(DESTROY(arp);) + IF_NO_ARC([arp release];) return nil; } @@ -1644,7 +1644,7 @@ recheck: if (NO == isEnum) { [self log: @"messed up NS_ENUM/NS_OPTIONS declaration"]; - [arp drain]; + IF_NO_ARC([arp release];) return nil; } } @@ -2008,8 +2008,8 @@ another: { if (buffer[pos] == ')' || buffer[pos] == ',') { - [arp drain]; - return declarations; + IF_NO_ARC(declarations = [declarations retain]; [arp release];) + return AUTORELEASE(declarations); } else { @@ -2131,7 +2131,6 @@ another: } DESTROY(comment); - [arp drain]; if (inArgList == NO) { /* @@ -2149,11 +2148,13 @@ another: { [self log: @"parse declaration with no name - %@", d]; } + IF_NO_ARC([arp release];) return nil; } } [self setStandards: declarations]; - return declarations; + IF_NO_ARC(declarations = [declarations retain]; [arp release];) + return AUTORELEASE(declarations); } else { @@ -2161,7 +2162,7 @@ another: } fail: DESTROY(comment); - [arp drain]; + IF_NO_ARC([arp release];) return nil; } From 91124f324702539dc6ec2e63d3f349527b969857 Mon Sep 17 00:00:00 2001 From: rfm Date: Wed, 20 Nov 2024 17:46:08 +0000 Subject: [PATCH 05/16] Fix boundary error in replacement function --- Source/GSICUString.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/GSICUString.m b/Source/GSICUString.m index aace61730..c8f45b25d 100644 --- a/Source/GSICUString.m +++ b/Source/GSICUString.m @@ -166,7 +166,7 @@ UTextNSMutableStringReplace(UText *ut, // Setting the chunk length to 0 here forces UTextNSStringAccess to fetch // the data from the string object. ut->chunkLength = 0; - UTextNSStringAccess(ut, r.location + [replacement length] + 1, TRUE); + UTextNSStringAccess(ut, r.location + [replacement length], TRUE); ut->chunkOffset++; [replacement release]; From 7b77a2074a83dc50e16a94e1b1d96a020247a846 Mon Sep 17 00:00:00 2001 From: rfm Date: Wed, 20 Nov 2024 17:46:35 +0000 Subject: [PATCH 06/16] Workaround to avoid memory leak in ICU functions if we set NULL as destination. --- Source/NSRegularExpression.m | 103 +++++++++++++++-------------------- 1 file changed, 44 insertions(+), 59 deletions(-) diff --git a/Source/NSRegularExpression.m b/Source/NSRegularExpression.m index fb407496e..80c26c37f 100644 --- a/Source/NSRegularExpression.m +++ b/Source/NSRegularExpression.m @@ -868,35 +868,29 @@ prepareResult(NSRegularExpression *regex, { // FIXME: We're computing a value that is most likely ignored in an // expensive way. - NSInteger results = [self numberOfMatchesInString: string - options: opts - range: range]; - UErrorCode s = 0; - UText txt = UTEXT_INITIALIZER; - UText replacement = UTEXT_INITIALIZER; - GSUTextString *ret = [GSUTextString new]; - URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); - UText *output = NULL; + NSInteger results = [self numberOfMatchesInString: string + options: opts + range: range]; + UErrorCode s = 0; + UText dst = UTEXT_INITIALIZER; + UText txt = UTEXT_INITIALIZER; + UText replacement = UTEXT_INITIALIZER; + NSMutableString *ms = [NSMutableString string]; + URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); UTextInitWithNSString(&replacement, template); + UTextInitWithNSMutableString(&dst, ms); - output = uregex_replaceAllUText(r, &replacement, NULL, &s); + uregex_replaceAllUText(r, &replacement, &dst, &s); + uregex_close(r); + utext_close(&replacement); + utext_close(&txt); + utext_close(&dst); if (0 != s) { - uregex_close(r); - utext_close(&replacement); - utext_close(&txt); - DESTROY(ret); return 0; } - utext_clone(&ret->txt, output, TRUE, TRUE, &s); - [string setString: ret]; - RELEASE(ret); - uregex_close(r); - - utext_close(&txt); - utext_close(output); - utext_close(&replacement); + [string setString: ms]; return results; } @@ -905,31 +899,26 @@ prepareResult(NSRegularExpression *regex, range: (NSRange)range withTemplate: (NSString*)template { - UErrorCode s = 0; - UText txt = UTEXT_INITIALIZER; - UText replacement = UTEXT_INITIALIZER; - UText *output = NULL; - GSUTextString *ret = [GSUTextString new]; - URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); + UErrorCode s = 0; + UText dst = UTEXT_INITIALIZER; + UText txt = UTEXT_INITIALIZER; + UText replacement = UTEXT_INITIALIZER; + NSMutableString *ms = [NSMutableString string]; + URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); UTextInitWithNSString(&replacement, template); + UTextInitWithNSMutableString(&dst, ms); - output = uregex_replaceAllUText(r, &replacement, NULL, &s); + uregex_replaceAllUText(r, &replacement, &dst, &s); + uregex_close(r); + utext_close(&replacement); + utext_close(&txt); + utext_close(&dst); if (0 != s) { - uregex_close(r); - utext_close(&replacement); - utext_close(&txt); - DESTROY(ret); return nil; } - utext_clone(&ret->txt, output, TRUE, TRUE, &s); - uregex_close(r); - - utext_close(&txt); - utext_close(output); - utext_close(&replacement); - return AUTORELEASE(ret); + return ms; } - (NSString*) replacementStringForResult: (NSTextCheckingResult*)result @@ -937,13 +926,13 @@ prepareResult(NSRegularExpression *regex, offset: (NSInteger)offset template: (NSString*)template { - UErrorCode s = 0; - UText txt = UTEXT_INITIALIZER; - UText replacement = UTEXT_INITIALIZER; - UText *output = NULL; - GSUTextString *ret = [GSUTextString new]; - NSRange range = [result range]; - URegularExpression *r = setupRegex(regex, + UErrorCode s = 0; + UText dst = UTEXT_INITIALIZER; + UText txt = UTEXT_INITIALIZER; + UText replacement = UTEXT_INITIALIZER; + NSMutableString *ms = [NSMutableString string]; + NSRange range = [result range]; + URegularExpression *r = setupRegex(regex, [string substringWithRange: range], &txt, 0, @@ -951,22 +940,18 @@ prepareResult(NSRegularExpression *regex, 0); UTextInitWithNSString(&replacement, template); + UTextInitWithNSMutableString(&dst, ms); - output = uregex_replaceFirstUText(r, &replacement, NULL, &s); - if (0 != s) - { - uregex_close(r); - utext_close(&replacement); - utext_close(&txt); - DESTROY(ret); - return nil; - } - utext_clone(&ret->txt, output, TRUE, TRUE, &s); - utext_close(output); + uregex_replaceFirstUText(r, &replacement, &dst, &s); uregex_close(r); + utext_close(&dst); utext_close(&txt); utext_close(&replacement); - return AUTORELEASE(ret); + if (0 != s) + { + return nil; + } + return ms; } #else - (NSUInteger) replaceMatchesInString: (NSMutableString*)string From 65ae40722ee420d462b5745352e329a84951fa78 Mon Sep 17 00:00:00 2001 From: rfm Date: Thu, 21 Nov 2024 14:35:54 +0000 Subject: [PATCH 07/16] Fix leak of autorelease pools --- Source/Additions/NSObject+GNUstepBase.m | 10 +++ Source/NSAutoreleasePool.m | 99 ++++++++++++++++--------- 2 files changed, 75 insertions(+), 34 deletions(-) diff --git a/Source/Additions/NSObject+GNUstepBase.m b/Source/Additions/NSObject+GNUstepBase.m index 8adcdd28e..b38af80ac 100644 --- a/Source/Additions/NSObject+GNUstepBase.m +++ b/Source/Additions/NSObject+GNUstepBase.m @@ -146,6 +146,11 @@ @end #if defined(GNUSTEP) + +@interface NSAutoreleasePool (NSThread) ++ (void) _endThread: (NSThread*)thread; +@end + struct exitLink { struct exitLink *next; id obj; // Object to release or class for atExit @@ -235,6 +240,11 @@ handleExit() { GSUnregisterCurrentThread(); } + else + { + [[NSAutoreleasePool currentPool] dealloc]; + [NSAutoreleasePool _endThread: GSCurrentThread()]; + } isExiting = NO; } diff --git a/Source/NSAutoreleasePool.m b/Source/NSAutoreleasePool.m index ed8c69f7a..9a37e52c8 100644 --- a/Source/NSAutoreleasePool.m +++ b/Source/NSAutoreleasePool.m @@ -43,6 +43,8 @@ +#define LOG_LIFETIME 0 + /* When this is `NO', autoreleased objects are never actually recorded in an NSAutoreleasePool, and are not sent a `release' message. Thus memory for objects use grows, and grows, and... */ @@ -73,10 +75,10 @@ static unsigned pool_number_warning_threshold = 10000; already alloc'ed. The cache is kept in the autorelease_thread_var structure, which is an ivar of NSThread. */ -static id pop_pool_from_cache (struct autorelease_thread_vars *tv); +static id pop_pool_from_cache(struct autorelease_thread_vars *tv); static inline void -free_pool_cache (struct autorelease_thread_vars *tv) +free_pool_cache(struct autorelease_thread_vars *tv) { while (tv->pool_cache_count) { @@ -94,7 +96,7 @@ free_pool_cache (struct autorelease_thread_vars *tv) } static inline void -init_pool_cache (struct autorelease_thread_vars *tv) +init_pool_cache(struct autorelease_thread_vars *tv) { tv->pool_cache_size = 32; tv->pool_cache_count = 0; @@ -103,11 +105,11 @@ init_pool_cache (struct autorelease_thread_vars *tv) } static void -push_pool_to_cache (struct autorelease_thread_vars *tv, id p) +push_pool_to_cache(struct autorelease_thread_vars *tv, id p) { if (!tv->pool_cache) { - init_pool_cache (tv); + init_pool_cache(tv); } else if (tv->pool_cache_count == tv->pool_cache_size) { @@ -119,7 +121,7 @@ push_pool_to_cache (struct autorelease_thread_vars *tv, id p) } static id -pop_pool_from_cache (struct autorelease_thread_vars *tv) +pop_pool_from_cache(struct autorelease_thread_vars *tv) { return tv->pool_cache[--(tv->pool_cache_count)]; } @@ -138,10 +140,11 @@ pop_pool_from_cache (struct autorelease_thread_vars *tv) + (id) allocWithZone: (NSZone*)zone { struct autorelease_thread_vars *tv = ARP_THREAD_VARS; + NSAutoreleasePool *p; if (tv->pool_cache_count) { - NSAutoreleasePool *p = pop_pool_from_cache (tv); + p = pop_pool_from_cache(tv); /* When we cache a 'deallocated' pool, we set its _released_count to * UINT_MAX, so when we rtrieve it fromm the cache we must increment @@ -154,7 +157,14 @@ pop_pool_from_cache (struct autorelease_thread_vars *tv) } return p; } - return NSAllocateObject (self, 0, zone); + p = (NSAutoreleasePool*)NSAllocateObject (self, 0, zone); + +#if LOG_LIFETIME + fprintf(stderr, "*** %p autorelease pool allocated in %p\n", + p, GSCurrentThread()); +#endif + + return p; } + (id) new @@ -174,6 +184,37 @@ pop_pool_from_cache (struct autorelease_thread_vars *tv) return (*initImp)(arp, @selector(init)); } +- (void) _emptyChild +{ + /* If there are NSAutoreleasePool below us in the list of + * NSAutoreleasePools, then deallocate them also. + * The (only) way we could get in this situation (in correctly + * written programs, that don't release NSAutoreleasePools in + * weird ways), is if an exception threw us up the stack. + * However, if a program has leaked pools we may be deallocating + * a pool with LOTS of children. To avoid stack overflow we + * therefore deallocate children starting with the oldest first. + */ + if (nil != _child) + { + NSAutoreleasePool *pool = _child; + + /* Find other end of linked list ... oldest child. + */ + while (nil != pool->_child) + { + pool = pool->_child; + } + /* Deallocate the children in the list. + */ + while (pool != self) + { + pool = pool->_parent; + [pool->_child dealloc]; + } + } +} + #ifdef ARC_RUNTIME - (id) init @@ -188,6 +229,7 @@ pop_pool_from_cache (struct autorelease_thread_vars *tv) { NSAutoreleasePool *pool = _parent; + _child = _parent->_child; while (nil != pool) { level++; @@ -240,8 +282,13 @@ pop_pool_from_cache (struct autorelease_thread_vars *tv) - (void) emptyPool { struct autorelease_thread_vars *tv = ARP_THREAD_VARS; - tv->current_pool = self; + + if (nil != _child) + { + [self _emptyChild]; + } objc_autoreleasePoolPop(_released); + tv->current_pool = self; } /** * Indicate to the runtime that we have an ARC-compatible implementation of @@ -282,11 +329,13 @@ pop_pool_from_cache (struct autorelease_thread_vars *tv) { struct autorelease_thread_vars *tv = ARP_THREAD_VARS; unsigned level = 0; + _parent = tv->current_pool; if (_parent) { NSAutoreleasePool *pool = _parent; + _child = _parent->_child; while (nil != pool) { level++; @@ -449,32 +498,9 @@ pop_pool_from_cache (struct autorelease_thread_vars *tv) { volatile struct autorelease_array_list *released; - /* If there are NSAutoreleasePool below us in the list of - * NSAutoreleasePools, then deallocate them also. - * The (only) way we could get in this situation (in correctly - * written programs, that don't release NSAutoreleasePools in - * weird ways), is if an exception threw us up the stack. - * However, if a program has leaked pools we may be deallocating - * a pool with LOTS of children. To avoid stack overflow we - * therefore deallocate children starting with the oldest first. - */ if (nil != _child) { - NSAutoreleasePool *pool = _child; - - /* Find other end of linked list ... oldest child. - */ - while (nil != pool->_child) - { - pool = pool->_child; - } - /* Deallocate the children in the list. - */ - while (pool != self) - { - pool = pool->_parent; - [pool->_child dealloc]; - } + [self _emptyChild]; } /* Take the object out of the released list just before releasing it, @@ -593,13 +619,14 @@ pop_pool_from_cache (struct autorelease_thread_vars *tv) _released_count = UINT_MAX; /* Don't deallocate ourself, just save us for later use. */ - push_pool_to_cache (tv, self); + push_pool_to_cache(tv, self); GSNOSUPERDEALLOC; } - (void) _reallyDealloc { struct autorelease_array_list *a; + for (a = _released_head; a;) { void *n = a->next; @@ -607,6 +634,10 @@ pop_pool_from_cache (struct autorelease_thread_vars *tv) a = n; } _released = _released_head = 0; +#if LOG_LIFETIME + fprintf(stderr, "*** %p autorelease pool really dealloc\n", self); +#endif + [super dealloc]; } From c224c63c2f539fef18f98dbafbe1591b0f1e53b7 Mon Sep 17 00:00:00 2001 From: rfm Date: Thu, 21 Nov 2024 14:36:16 +0000 Subject: [PATCH 08/16] Fix leak --- Tools/AGSParser.m | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Tools/AGSParser.m b/Tools/AGSParser.m index f8458e68a..dab45e4fe 100644 --- a/Tools/AGSParser.m +++ b/Tools/AGSParser.m @@ -2487,7 +2487,7 @@ fail: */ [self skipUnit]; DESTROY(comment); - [arp drain]; + IF_NO_ARC([arp release];) return [NSMutableDictionary dictionary]; } else @@ -2526,13 +2526,13 @@ fail: DESTROY(unitName); DESTROY(comment); - [arp drain]; + IF_NO_ARC([arp release];) return dict; fail: DESTROY(unitName); DESTROY(comment); - [arp drain]; + IF_NO_ARC([arp release];) return nil; } @@ -2692,13 +2692,13 @@ fail: DESTROY(unitName); DESTROY(comment); - [arp drain]; + IF_NO_ARC([arp release];) return dict; fail: DESTROY(unitName); DESTROY(comment); - [arp drain]; + IF_NO_ARC([arp release];) return nil; } @@ -3052,7 +3052,7 @@ fail: - (NSMutableDictionary*) parseMethodIsDeclaration: (BOOL)flag { - CREATE_AUTORELEASE_POOL(arp); + IF_NO_ARC(CREATE_AUTORELEASE_POOL(arp);) NSMutableDictionary *method; NSMutableString *mname; NSString *token; @@ -3340,14 +3340,14 @@ fail: } DESTROY(itemName); - [arp drain]; + IF_NO_ARC([arp release];) IF_NO_ARC([method autorelease];) return method; fail: DESTROY(itemName); DESTROY(comment); - [arp drain]; + IF_NO_ARC([arp release];) RELEASE(method); return nil; } @@ -4290,7 +4290,7 @@ countAttributes(NSSet *keys, NSDictionary *a) NSDictionary *methods = nil; NSMutableDictionary *dict; NSMutableDictionary *d; - CREATE_AUTORELEASE_POOL(arp); + IF_NO_ARC(CREATE_AUTORELEASE_POOL(arp);) dict = [[NSMutableDictionary alloc] initWithCapacity: 8]; @@ -4381,14 +4381,14 @@ countAttributes(NSSet *keys, NSDictionary *a) DESTROY(unitName); DESTROY(comment); - [arp drain]; + IF_NO_ARC([arp release];) IF_NO_ARC([dict autorelease];) return dict; fail: DESTROY(unitName); DESTROY(comment); - [arp drain]; + IF_NO_ARC([arp release];) RELEASE(dict); return nil; } @@ -4818,7 +4818,7 @@ fail: unichar *inptr; unichar *outptr; NSMutableArray *a; - CREATE_AUTORELEASE_POOL(arp); + IF_NO_ARC(CREATE_AUTORELEASE_POOL(arp);) contents = [NSString stringWithContentsOfFile: fileName]; length = [contents length]; @@ -4912,7 +4912,7 @@ fail: buffer = [data mutableBytes]; pos = 0; ASSIGN(lines, [NSArray arrayWithArray: a]); - [arp drain]; + IF_NO_ARC([arp release];) IF_NO_ARC([data autorelease];) } From 37e07116a0d5be0d3ae8f6ac5ebc38002ed5083f Mon Sep 17 00:00:00 2001 From: rfm Date: Thu, 21 Nov 2024 14:48:18 +0000 Subject: [PATCH 09/16] Document latest changes --- ChangeLog | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ChangeLog b/ChangeLog index 99d3eedbd..dfcede6ad 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2024-11-21 Richard Frith-Macdonald + + * NSRegularExpression: avoid leaks in ICU code by calling replacement + methods with a non-NULL destination. + * NSAutoreleasePool: fix leaks with libobjc2 ARC methods when a pool + is in a context that we leave due to an exception. + 2024-11-19 Richard Frith-Macdonald * GSMime: fixed buffer overrun in rare circumstances when decoding From 087716fffa589416eeabe47cb337142c4e7d4e98 Mon Sep 17 00:00:00 2001 From: rfm Date: Thu, 21 Nov 2024 18:18:06 +0000 Subject: [PATCH 10/16] Improve dagnostic method for memory leaks --- ChangeLog | 2 + Headers/GNUstepBase/NSObject+GNUstepBase.h | 6 + Source/Additions/NSObject+GNUstepBase.m | 287 +++++++++++++++++---- 3 files changed, 240 insertions(+), 55 deletions(-) diff --git a/ChangeLog b/ChangeLog index dfcede6ad..678691bfa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,8 @@ methods with a non-NULL destination. * NSAutoreleasePool: fix leaks with libobjc2 ARC methods when a pool is in a context that we leave due to an exception. + * NSObject: add -trackOwnership to log the retain/release/dealloc of + in individual instance. 2024-11-19 Richard Frith-Macdonald diff --git a/Headers/GNUstepBase/NSObject+GNUstepBase.h b/Headers/GNUstepBase/NSObject+GNUstepBase.h index fdaee5240..fb88ff74e 100644 --- a/Headers/GNUstepBase/NSObject+GNUstepBase.h +++ b/Headers/GNUstepBase/NSObject+GNUstepBase.h @@ -295,8 +295,14 @@ extern "C" { */ + (BOOL) shouldCleanUp; +/** Turns on tracking of retain/release for instances of the receiver. + */ + (void) trackOwnership; +/** Turns on tracking of retain/release for the receiver. + */ +- (void) trackOwnership; + @end /* Macro to take an autoreleased object and either make it immutable or diff --git a/Source/Additions/NSObject+GNUstepBase.m b/Source/Additions/NSObject+GNUstepBase.m index b38af80ac..ae6d42676 100644 --- a/Source/Additions/NSObject+GNUstepBase.m +++ b/Source/Additions/NSObject+GNUstepBase.m @@ -433,96 +433,191 @@ handleExit() struct trackLink { struct trackLink *next; - Class cls; // Class whose instaances are tracked. + id object; // Instance or Class being tracked. + IMP dealloc; // Original -dealloc implementation IMP release; // Original -release implementation IMP retain; // Original -retain implementation + BOOL global; // If all instance are tracked. }; static struct trackLink *tracked = 0; static gs_mutex_t trackLock = GS_MUTEX_INIT_STATIC; -- (void) _replacementRelease +static inline struct trackLink * +find(id o) { - struct trackLink *l; - Class c = object_getClass(self); - IMP release = 0; - unsigned rc; + struct trackLink *l = tracked; - GS_MUTEX_LOCK(trackLock); - l = tracked; while (l) { - if (l->cls == c) + if (l->object == o) { - release = l->release; - break; + return l; } l = l->next; } + return NULL; +} + +/* Lookup the object in the tracking list. + * If found as a tracked instance or found as an instance of a class for which + * all instances are tracked, return YES. Otherwise return NO (should not log). + */ +static BOOL +findMethods(id o, IMP *dea, IMP *rel, IMP *ret) +{ + struct trackLink *l; + Class c; + + GS_MUTEX_LOCK(trackLock); + l = find(o); + if (l) + { + *dea = l->dealloc; + *rel = l->release; + *ret = l->retain; + GS_MUTEX_UNLOCK(trackLock); + return YES; + } + c = object_getClass(o); + l = find((id)c); + if (l) + { + BOOL all; + + *dea = l->dealloc; + *rel = l->release; + *ret = l->retain; + all = l->global; + GS_MUTEX_UNLOCK(trackLock); + return all; + } GS_MUTEX_UNLOCK(trackLock); - rc = (unsigned)[self retainCount]; - NSLog(@"-[%p release] %u->%u at %@", - self, rc, rc-1, [NSThread callStackSymbols]); - (*release)(self, _cmd); + /* Should never happen because we don't remove class entries, but I suppose + * someone could call the replacement methods directly. + */ + *dea = [c instanceMethodForSelector: @selector(dealloc)]; + *rel = [c instanceMethodForSelector: @selector(release)]; + *ret = [c instanceMethodForSelector: @selector(retain)]; + return NO; +} + +- (void) _replacementDealloc +{ + IMP dealloc = 0; + IMP retain = 0; + IMP release = 0; + + if (findMethods(self, &dealloc, &release, &retain) == NO) + { + /* Not a tracked instance ... dealloc without logging. + */ + (*dealloc)(self, _cmd); + } + else + { + struct trackLink *l; + + /* If there's a link for tracking this specific instance, remove it. + */ + GS_MUTEX_LOCK(trackLock); + if ((l = tracked) != 0) + { + if (l->object == self) + { + tracked = l->next; + free(l); + } + else + { + struct trackLink *n; + + while ((n = l->next) != 0) + { + if (n->object == self) + { + l->next = n->next; + free(n); + break; + } + l = n; + } + } + } + GS_MUTEX_UNLOCK(trackLock); + NSLog(@"Tracking ownership -[%p dealloc] at %@", + self, [NSThread callStackSymbols]); + (*dealloc)(self, _cmd); + } +} +- (void) _replacementRelease +{ + IMP dealloc = 0; + IMP retain = 0; + IMP release = 0; + + if (findMethods(self, &dealloc, &release, &retain) == NO) + { + /* Not a tracked instance ... release without logging. + */ + (*release)(self, _cmd); + } + else + { + unsigned rc; + + rc = (unsigned)[self retainCount]; + NSLog(@"Tracking ownership -[%p release] %u->%u at %@", + self, rc, rc-1, [NSThread callStackSymbols]); + (*release)(self, _cmd); + } } - (id) _replacementRetain { - struct trackLink *l; - Class c = object_getClass(self); - IMP retain = 0; - id result; - unsigned rc; + IMP dealloc = 0; + IMP retain = 0; + IMP release = 0; + id result; - GS_MUTEX_LOCK(trackLock); - l = tracked; - while (l) + if (findMethods(self, &dealloc, &release, &retain) == NO) { - if (l->cls == c) - { - retain = l->retain; - break; - } - l = l->next; + /* Not a tracked instance ... retain without logging. + */ + result = (*retain)(self, _cmd); + } + else + { + unsigned rc; + + rc = (unsigned)[self retainCount]; + result = (*retain)(self, _cmd); + NSLog(@"Tracking ownership -[%p retain] %u->%u at %@", + self, rc, (unsigned)[self retainCount], [NSThread callStackSymbols]); } - GS_MUTEX_UNLOCK(trackLock); - rc = (unsigned)[self retainCount]; - result = (*retain)(self, _cmd); - NSLog(@"-[%p retain] %u->%u at %@", - self, rc, (unsigned)[self retainCount], [NSThread callStackSymbols]); return result; } -+ (void) trackOwnership +static struct trackLink* +makeLinkForClass(Class c) { + Method replacementDealloc; Method replacementRelease; Method replacementRetain; - Class c = object_getClass(self); - struct trackLink *l; - - if (class_isMetaClass(c)) - { - c = self; - } - - GS_MUTEX_LOCK(trackLock); - l = tracked; - while (l) - { - if (l->cls == c) - { - GS_MUTEX_UNLOCK(trackLock); - return; - } - l = l->next; - } + struct trackLink *l; + replacementDealloc = class_getInstanceMethod([NSObject class], + @selector(_replacementDealloc)); replacementRelease = class_getInstanceMethod([NSObject class], @selector(_replacementRelease)); replacementRetain = class_getInstanceMethod([NSObject class], @selector(_replacementRetain)); l = (struct trackLink*)malloc(sizeof(struct trackLink)); - l->cls = c; + l->object = c; + l->dealloc = class_getMethodImplementation(c, @selector(dealloc)); + class_replaceMethod(c, @selector(dealloc), + method_getImplementation(replacementDealloc), + method_getTypeEncoding(replacementDealloc)); l->release = class_getMethodImplementation(c, @selector(release)); class_replaceMethod(c, @selector(release), method_getImplementation(replacementRelease), @@ -531,9 +626,91 @@ static gs_mutex_t trackLock = GS_MUTEX_INIT_STATIC; class_replaceMethod(c, @selector(retain), method_getImplementation(replacementRetain), method_getTypeEncoding(replacementRetain)); + + return l; +} + ++ (void) trackOwnership +{ + Class c = self; + struct trackLink *l; + + NSAssert(NO == class_isMetaClass(object_getClass(self)), + NSInternalInconsistencyException); + + GS_MUTEX_LOCK(trackLock); + if ((l = find((id)c)) != 0) + { + /* Class already tracked. Set it so all instances are logged. + */ + l->global = YES; + GS_MUTEX_UNLOCK(trackLock); + return; + } + + l = makeLinkForClass(c); + l->global = YES; l->next = tracked; tracked = l; GS_MUTEX_UNLOCK(trackLock); + NSLog(@"Tracking ownership started for class %p at %@", + self, [NSThread callStackSymbols]); +} + +- (void) trackOwnership +{ + Class c = object_getClass(self); + struct trackLink *l; + struct trackLink *lc; + struct trackLink *li; + + NSAssert(NO == class_isMetaClass(c), NSInternalInconsistencyException); + + GS_MUTEX_LOCK(trackLock); + if ((l = find(self)) != 0) + { + /* Instance already tracked. + */ + GS_MUTEX_UNLOCK(trackLock); + return; + } + + if ((l = find(c)) != 0) + { + /* The class already has tracking set up. + */ + if (l->global) + { + /* All instances are logged, so we have nothing to do. + */ + GS_MUTEX_UNLOCK(trackLock); + return; + } + lc = l; + } + else + { + /* Set this class up for tracking individual instances. + */ + lc = makeLinkForClass(c); + lc->global = NO; + lc->next = tracked; + tracked = lc; + } + + /* Now set up a record to track this one instance. + */ + li = (struct trackLink*)malloc(sizeof(struct trackLink)); + li->object = self; + li->global = NO; + li->dealloc = lc->dealloc; + li->release = lc->release; + li->retain = lc->retain; + li->next = tracked; + tracked = li; + GS_MUTEX_UNLOCK(trackLock); + NSLog(@"Tracking ownership started for instance %p at %@", + self, [NSThread callStackSymbols]); } @end From 4c287f20b3a946e63db30857686386586e998d7c Mon Sep 17 00:00:00 2001 From: rfm Date: Thu, 21 Nov 2024 21:05:34 +0000 Subject: [PATCH 11/16] More regular expression leak fix changes ... use older code (plus fixes). --- ChangeLog | 4 +- Source/NSRegularExpression.m | 273 +++++++++++++++++++++++++---------- 2 files changed, 195 insertions(+), 82 deletions(-) diff --git a/ChangeLog b/ChangeLog index 678691bfa..0bac5c189 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,7 +1,7 @@ 2024-11-21 Richard Frith-Macdonald - * NSRegularExpression: avoid leaks in ICU code by calling replacement - methods with a non-NULL destination. + * NSRegularExpression: avoid leaks in ICU code by calling more + primitive methods. * NSAutoreleasePool: fix leaks with libobjc2 ARC methods when a pool is in a context that we leave due to an exception. * NSObject: add -trackOwnership to log the retain/release/dealloc of diff --git a/Source/NSRegularExpression.m b/Source/NSRegularExpression.m index 80c26c37f..1482fb34a 100644 --- a/Source/NSRegularExpression.m +++ b/Source/NSRegularExpression.m @@ -35,9 +35,19 @@ * won't work because libicu internally renames all entry points with some cpp * magic. */ +#if !defined(HAVE_UREGEX_OPENUTEXT) #if U_ICU_VERSION_MAJOR_NUM > 4 || (U_ICU_VERSION_MAJOR_NUM == 4 && U_ICU_VERSION_MINOR_NUM >= 4) || defined(HAVE_ICU_H) #define HAVE_UREGEX_OPENUTEXT 1 #endif +#endif + +/* Until the uregex_replaceAllUText() and uregex_replaceFirstUText() work + * without leaking memory, we can't use them :-( + * Preoblem exists on Ubuntu in 2024 with icu-74.2 + */ +#if defined(HAVE_UREGEX_OPENUTEXT) +#undef HAVE_UREGEX_OPENUTEXT +#endif #define NSRegularExpressionWorks @@ -158,7 +168,8 @@ NSRegularExpressionOptionsToURegexpFlags(NSRegularExpressionOptions opts) NSDictionary *userInfo; NSString *description; - description = [NSString stringWithFormat: @"The value “%@” is invalid.", aPattern]; + description = [NSString + stringWithFormat: @"The value “%@” is invalid.", aPattern]; userInfo = [NSDictionary dictionaryWithObjectsAndKeys: aPattern, @"NSInvalidValue", @@ -245,11 +256,46 @@ NSRegularExpressionOptionsToURegexpFlags(NSRegularExpressionOptions opts) } #endif + // Raise an NSInvalidArgumentException to match macOS behaviour. + if (!aPattern) + { + NSException *exp; + + exp = [NSException exceptionWithName: NSInvalidArgumentException + reason: @"nil argument" + userInfo: nil]; + RELEASE(self); + [exp raise]; + } + [aPattern getCharacters: buffer range: NSMakeRange(0, length)]; regex = uregex_open(buffer, length, flags, &pe, &s); if (U_FAILURE(s)) { - // FIXME: Do something sensible with the error parameter. + /* Match macOS behaviour if the pattern is invalid. + * Example: + * Domain=NSCocoaErrorDomain + * Code=2048 "The value “” is invalid." + * UserInfo={NSInvalidValue=} + */ + if (e) + { + NSDictionary *userInfo; + NSString *description; + + description = [NSString + stringWithFormat: @"The value “%@” is invalid.", aPattern]; + + userInfo = [NSDictionary dictionaryWithObjectsAndKeys: + aPattern, @"NSInvalidValue", + description, NSLocalizedDescriptionKey, + nil]; + + *e = [NSError errorWithDomain: NSCocoaErrorDomain + code: NSFormattingError + userInfo: userInfo]; + } + DESTROY(self); return self; } @@ -868,29 +914,35 @@ prepareResult(NSRegularExpression *regex, { // FIXME: We're computing a value that is most likely ignored in an // expensive way. - NSInteger results = [self numberOfMatchesInString: string - options: opts - range: range]; - UErrorCode s = 0; - UText dst = UTEXT_INITIALIZER; - UText txt = UTEXT_INITIALIZER; - UText replacement = UTEXT_INITIALIZER; - NSMutableString *ms = [NSMutableString string]; - URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); + NSInteger results = [self numberOfMatchesInString: string + options: opts + range: range]; + UErrorCode s = 0; + UText txt = UTEXT_INITIALIZER; + UText replacement = UTEXT_INITIALIZER; + GSUTextString *ret = [GSUTextString new]; + URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); + UText *output = NULL; UTextInitWithNSString(&replacement, template); - UTextInitWithNSMutableString(&dst, ms); - uregex_replaceAllUText(r, &replacement, &dst, &s); - uregex_close(r); - utext_close(&replacement); - utext_close(&txt); - utext_close(&dst); + output = uregex_replaceAllUText(r, &replacement, NULL, &s); if (0 != s) { + uregex_close(r); + utext_close(&replacement); + utext_close(&txt); + DESTROY(ret); return 0; } - [string setString: ms]; + utext_clone(&ret->txt, output, TRUE, TRUE, &s); + [string setString: ret]; + RELEASE(ret); + uregex_close(r); + + utext_close(&txt); + utext_close(output); + utext_close(&replacement); return results; } @@ -899,26 +951,31 @@ prepareResult(NSRegularExpression *regex, range: (NSRange)range withTemplate: (NSString*)template { - UErrorCode s = 0; - UText dst = UTEXT_INITIALIZER; - UText txt = UTEXT_INITIALIZER; - UText replacement = UTEXT_INITIALIZER; - NSMutableString *ms = [NSMutableString string]; - URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); + UErrorCode s = 0; + UText txt = UTEXT_INITIALIZER; + UText replacement = UTEXT_INITIALIZER; + UText *output = NULL; + GSUTextString *ret = [GSUTextString new]; + URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); UTextInitWithNSString(&replacement, template); - UTextInitWithNSMutableString(&dst, ms); - uregex_replaceAllUText(r, &replacement, &dst, &s); - uregex_close(r); - utext_close(&replacement); - utext_close(&txt); - utext_close(&dst); + output = uregex_replaceAllUText(r, &replacement, NULL, &s); if (0 != s) { + uregex_close(r); + utext_close(&replacement); + utext_close(&txt); + DESTROY(ret); return nil; } - return ms; + utext_clone(&ret->txt, output, TRUE, TRUE, &s); + uregex_close(r); + + utext_close(&txt); + utext_close(output); + utext_close(&replacement); + return AUTORELEASE(ret); } - (NSString*) replacementStringForResult: (NSTextCheckingResult*)result @@ -926,13 +983,13 @@ prepareResult(NSRegularExpression *regex, offset: (NSInteger)offset template: (NSString*)template { - UErrorCode s = 0; - UText dst = UTEXT_INITIALIZER; - UText txt = UTEXT_INITIALIZER; - UText replacement = UTEXT_INITIALIZER; - NSMutableString *ms = [NSMutableString string]; - NSRange range = [result range]; - URegularExpression *r = setupRegex(regex, + UErrorCode s = 0; + UText txt = UTEXT_INITIALIZER; + UText replacement = UTEXT_INITIALIZER; + UText *output = NULL; + GSUTextString *ret = [GSUTextString new]; + NSRange range = [result range]; + URegularExpression *r = setupRegex(regex, [string substringWithRange: range], &txt, 0, @@ -940,18 +997,22 @@ prepareResult(NSRegularExpression *regex, 0); UTextInitWithNSString(&replacement, template); - UTextInitWithNSMutableString(&dst, ms); - uregex_replaceFirstUText(r, &replacement, &dst, &s); - uregex_close(r); - utext_close(&dst); - utext_close(&txt); - utext_close(&replacement); + output = uregex_replaceFirstUText(r, &replacement, NULL, &s); if (0 != s) { + uregex_close(r); + utext_close(&replacement); + utext_close(&txt); + DESTROY(ret); return nil; } - return ms; + utext_clone(&ret->txt, output, TRUE, TRUE, &s); + utext_close(output); + uregex_close(r); + utext_close(&txt); + utext_close(&replacement); + return AUTORELEASE(ret); } #else - (NSUInteger) replaceMatchesInString: (NSMutableString*)string @@ -964,31 +1025,51 @@ prepareResult(NSRegularExpression *regex, NSInteger results = [self numberOfMatchesInString: string options: opts range: range]; - UErrorCode s = 0; - uint32_t length = [string length]; - uint32_t replLength = [template length]; - unichar replacement[replLength]; - int32_t outLength; - unichar *output; - NSString *out; - URegularExpression *r; - TEMP_BUFFER(buffer, length); + if (results > 0) + { + UErrorCode s = 0; + uint32_t length = [string length]; + uint32_t replLength = [template length]; + unichar replacement[replLength]; + int32_t outLength; + URegularExpression *r; + TEMP_BUFFER(buffer, length); - r = setupRegex(regex, string, buffer, length, opts, range, 0); - [template getCharacters: replacement range: NSMakeRange(0, replLength)]; + r = setupRegex(regex, string, buffer, length, opts, range, 0); + [template getCharacters: replacement range: NSMakeRange(0, replLength)]; - outLength = uregex_replaceAll(r, replacement, replLength, NULL, 0, &s); + outLength = uregex_replaceAll(r, replacement, replLength, NULL, 0, &s); + if (0 == s || U_BUFFER_OVERFLOW_ERROR == s) + { + unichar *output; - s = 0; - output = NSZoneMalloc(0, outLength * sizeof(unichar)); - uregex_replaceAll(r, replacement, replLength, output, outLength, &s); - out = - [[NSString alloc] initWithCharactersNoCopy: output - length: outLength - freeWhenDone: YES]; - [string setString: out]; - RELEASE(out); + s = 0; // May have been set to a buffer overflow error + output = NSZoneMalloc(0, (outLength + 1) * sizeof(unichar)); + uregex_replaceAll(r, replacement, replLength, + output, outLength + 1, &s); + if (0 == s) + { + NSString *out; + + out = [[NSString alloc] initWithCharactersNoCopy: output + length: outLength + freeWhenDone: YES]; + [string setString: out]; + RELEASE(out); + } + else + { + NSZoneFree(0, output); + results = 0; + } + } + else + { + results = 0; + } + uregex_close(r); + } return results; } @@ -1003,20 +1084,36 @@ prepareResult(NSRegularExpression *regex, uint32_t replLength = [template length]; unichar replacement[replLength]; int32_t outLength; - unichar *output; + NSString *result = nil; TEMP_BUFFER(buffer, length); r = setupRegex(regex, string, buffer, length, opts, range, 0); [template getCharacters: replacement range: NSMakeRange(0, replLength)]; outLength = uregex_replaceAll(r, replacement, replLength, NULL, 0, &s); + if (0 == s || U_BUFFER_OVERFLOW_ERROR == s) + { + unichar *output; - s = 0; - output = NSZoneMalloc(0, outLength * sizeof(unichar)); - uregex_replaceAll(r, replacement, replLength, output, outLength, &s); - return AUTORELEASE([[NSString alloc] initWithCharactersNoCopy: output - length: outLength - freeWhenDone: YES]); + s = 0; // may have been set to a buffer overflow error + + output = NSZoneMalloc(0, (outLength + 1) * sizeof(unichar)); + uregex_replaceAll(r, replacement, replLength, output, outLength + 1, &s); + if (0 == s) + { + result = AUTORELEASE([[NSString alloc] + initWithCharactersNoCopy: output + length: outLength + freeWhenDone: YES]); + } + else + { + NSZoneFree(0, output); + } + } + + uregex_close(r); + return result; } - (NSString*) replacementStringForResult: (NSTextCheckingResult*)result @@ -1030,7 +1127,7 @@ prepareResult(NSRegularExpression *regex, uint32_t replLength = [template length]; unichar replacement[replLength]; int32_t outLength; - unichar *output; + NSString *str = nil; TEMP_BUFFER(buffer, range.length); r = setupRegex(regex, @@ -1043,12 +1140,28 @@ prepareResult(NSRegularExpression *regex, [template getCharacters: replacement range: NSMakeRange(0, replLength)]; outLength = uregex_replaceFirst(r, replacement, replLength, NULL, 0, &s); - s = 0; - output = NSZoneMalloc(0, outLength * sizeof(unichar)); - uregex_replaceFirst(r, replacement, replLength, output, outLength, &s); - return AUTORELEASE([[NSString alloc] initWithCharactersNoCopy: output - length: outLength - freeWhenDone: YES]); + if (0 == s || U_BUFFER_OVERFLOW_ERROR == s) + { + unichar *output; + + s = 0; + output = NSZoneMalloc(0, (outLength + 1) * sizeof(unichar)); + uregex_replaceFirst(r, replacement, replLength, + output, outLength + 1, &s); + if (0 == s) + { + str = AUTORELEASE([[NSString alloc] + initWithCharactersNoCopy: output + length: outLength + freeWhenDone: YES]); + } + else + { + NSZoneFree(0, output); + } + } + uregex_close(r); + return str; } #endif From 99bb50d2c09089d387a317c2d15aa68bed22ad65 Mon Sep 17 00:00:00 2001 From: rfm Date: Fri, 22 Nov 2024 06:04:38 +0000 Subject: [PATCH 12/16] avoid stack overflow --- ChangeLog | 7 +++++++ Tools/AGSIndex.m | 8 ++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0bac5c189..568ce499b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2024-11-22 Richard Frith-Macdonald + + * Tools/AGSIndex: (-makeRefs:) process child nodes in loop rather + than recursively ... reduces the depth of recursion to the depth of + nesting children rather than the number of children, avoiding stack + overflow for large projects. + 2024-11-21 Richard Frith-Macdonald * NSRegularExpression: avoid leaks in ICU code by calling more diff --git a/Tools/AGSIndex.m b/Tools/AGSIndex.m index ca4b1aac4..95fa84dd2 100644 --- a/Tools/AGSIndex.m +++ b/Tools/AGSIndex.m @@ -291,7 +291,6 @@ findKey(id refs, NSString *key, NSMutableArray *path, NSMutableArray *found) - (void) makeRefs: (GSXMLNode*)node { GSXMLNode *children = [node firstChild]; - GSXMLNode *next = [node next]; BOOL newUnit = NO; if ([node type] == XML_ELEMENT_NODE) @@ -541,9 +540,10 @@ findKey(id refs, NSString *key, NSMutableArray *path, NSMutableArray *found) } } - if (children != nil) + while (children) { [self makeRefs: children]; + children = [children next]; } if (newUnit == YES) { @@ -551,10 +551,6 @@ findKey(id refs, NSString *key, NSMutableArray *path, NSMutableArray *found) category = nil; classname = nil; } - if (next != nil) - { - [self makeRefs: next]; - } } /** From 021812f43d23f4d5e62b0d299f25801ca0b9eb5b Mon Sep 17 00:00:00 2001 From: rfm Date: Fri, 22 Nov 2024 06:18:44 +0000 Subject: [PATCH 13/16] Fix another leak --- ChangeLog | 2 ++ Source/GSHTTPURLHandle.m | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 568ce499b..fd7c16ab9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,8 @@ than recursively ... reduces the depth of recursion to the depth of nesting children rather than the number of children, avoiding stack overflow for large projects. + * Source/GSHTTPURLHandle.m: Fix leak of connection description strings + when reconnecting an existing handle. 2024-11-21 Richard Frith-Macdonald diff --git a/Source/GSHTTPURLHandle.m b/Source/GSHTTPURLHandle.m index babb489fe..10918e60c 100644 --- a/Source/GSHTTPURLHandle.m +++ b/Source/GSHTTPURLHandle.m @@ -1173,12 +1173,12 @@ debugWrite(GSHTTPURLHandle *handle, NSData *data) return; } - in = [[NSString alloc] initWithFormat: @"(%@:%@ <-- %@:%@)", + ASSIGN(in, [NSString stringWithFormat: @"(%@:%@ <-- %@:%@)", [sock socketLocalAddress], [sock socketLocalService], - [sock socketAddress], [sock socketService]]; - out = [[NSString alloc] initWithFormat: @"(%@:%@ --> %@:%@)", + [sock socketAddress], [sock socketService]]); + ASSIGN(out, [NSString stringWithFormat: @"(%@:%@ --> %@:%@)", [sock socketLocalAddress], [sock socketLocalService], - [sock socketAddress], [sock socketService]]; + [sock socketAddress], [sock socketService]]); if (debug) { From 7d4771ec2d282b2b0d9ef235fd47b2b118322666 Mon Sep 17 00:00:00 2001 From: rfm Date: Fri, 22 Nov 2024 06:27:57 +0000 Subject: [PATCH 14/16] add missing brackets --- Source/GSHTTPURLHandle.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/GSHTTPURLHandle.m b/Source/GSHTTPURLHandle.m index 10918e60c..0c7bfd1ff 100644 --- a/Source/GSHTTPURLHandle.m +++ b/Source/GSHTTPURLHandle.m @@ -1173,12 +1173,12 @@ debugWrite(GSHTTPURLHandle *handle, NSData *data) return; } - ASSIGN(in, [NSString stringWithFormat: @"(%@:%@ <-- %@:%@)", + ASSIGN(in, ([NSString stringWithFormat: @"(%@:%@ <-- %@:%@)", [sock socketLocalAddress], [sock socketLocalService], - [sock socketAddress], [sock socketService]]); - ASSIGN(out, [NSString stringWithFormat: @"(%@:%@ --> %@:%@)", + [sock socketAddress], [sock socketService]])); + ASSIGN(out, ([NSString stringWithFormat: @"(%@:%@ --> %@:%@)", [sock socketLocalAddress], [sock socketLocalService], - [sock socketAddress], [sock socketService]]); + [sock socketAddress], [sock socketService]])); if (debug) { From 168c2a4c90978457733b6c09321488c9306c9788 Mon Sep 17 00:00:00 2001 From: rfm Date: Sun, 24 Nov 2024 16:38:23 +0000 Subject: [PATCH 15/16] fix a leak establishign a connection --- ChangeLog | 5 +++++ Source/NSSocketPort.m | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index fd7c16ab9..599618164 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2024-11-24 Richard Frith-Macdonald + + * Source/NSSocketPort.m: fix leak of data object when establishing a + new connection. + 2024-11-22 Richard Frith-Macdonald * Tools/AGSIndex: (-makeRefs:) process child nodes in loop rather diff --git a/Source/NSSocketPort.m b/Source/NSSocketPort.m index 0682eaeed..cc36615a2 100644 --- a/Source/NSSocketPort.m +++ b/Source/NSSocketPort.m @@ -1247,7 +1247,8 @@ static Class runLoopClass; * first thing to do is send out port information (after setting * up a TLS session if necessary). */ - ASSIGN(cData, newDataWithEncodedPort(p)); + RELEASE(cData); + cData = newDataWithEncodedPort(p); cLength = 0; #if defined(HAVE_GNUTLS) From f2368086f4320fd60d1a4b55756179f5d584028d Mon Sep 17 00:00:00 2001 From: rfm Date: Wed, 27 Nov 2024 16:25:08 +0000 Subject: [PATCH 16/16] A bit of re-ordering to try to avoid premature initialisation of NSUserDefaults. --- Source/Additions/GSMime.m | 109 +++++++++++++++++--------------------- Source/NSBundle.m | 14 +++-- 2 files changed, 59 insertions(+), 64 deletions(-) diff --git a/Source/Additions/GSMime.m b/Source/Additions/GSMime.m index 1c7dfc140..48b42af1c 100644 --- a/Source/Additions/GSMime.m +++ b/Source/Additions/GSMime.m @@ -121,7 +121,6 @@ static Class NSArrayClass = 0; static Class NSStringClass = 0; static Class NSDataClass = 0; static Class documentClass = 0; -static Class headerClass = 0; static BOOL oldStyleFolding = NO; static NSString *Cte7bit = @"7bit"; @@ -888,6 +887,29 @@ wordData(NSString *word, BOOL *encoded) + (void) initialize { + NSMutableCharacterSet *m = [[NSMutableCharacterSet alloc] init]; + + [m formUnionWithCharacterSet: + [NSCharacterSet characterSetWithCharactersInString: + @".()<>@,;:[]\"\\"]]; + [m formUnionWithCharacterSet: + [NSCharacterSet whitespaceAndNewlineCharacterSet]]; + [m formUnionWithCharacterSet: + [NSCharacterSet controlCharacterSet]]; + [m formUnionWithCharacterSet: + [NSCharacterSet illegalCharacterSet]]; + rfc822Specials = [m copy]; + [[NSObject leakAt: &rfc822Specials] release]; + [m formUnionWithCharacterSet: + [NSCharacterSet characterSetWithCharactersInString: + @"/?="]]; + [m removeCharactersInString: @"."]; + rfc2045Specials = [m copy]; + [[NSObject leakAt: &rfc2045Specials] release]; + [m release]; + whitespace = RETAIN([NSCharacterSet whitespaceAndNewlineCharacterSet]); + [[NSObject leakAt: &whitespace] release]; + if (NSArrayClass == 0) { NSArrayClass = [NSArray class]; @@ -904,10 +926,6 @@ wordData(NSString *word, BOOL *encoded) { documentClass = [GSMimeDocument class]; } - if (headerClass == 0) - { - headerClass = [GSMimeHeader class]; - } } /** @@ -1745,9 +1763,9 @@ wordData(NSString *word, BOOL *encoded) /* * Set the header name. */ - info = [headerClass headerWithName: name - value: nil - parameters: nil]; + info = [GSMimeHeader headerWithName: name + value: nil + parameters: nil]; name = [info name]; /* @@ -3395,10 +3413,6 @@ static NSCharacterSet *tokenSet = nil; { documentClass = [GSMimeDocument class]; } - if (headerClass == 0) - { - headerClass = [GSMimeHeader class]; - } [[NSNotificationCenter defaultCenter] addObserver: self selector: @selector(_defaultsChanged:) name: NSUserDefaultsDidChangeNotification @@ -3607,7 +3621,7 @@ static NSCharacterSet *tokenSet = nil; NSEnumerator *e; NSString *k; - c = [headerClass allocWithZone: z]; + c = [[self class] allocWithZone: z]; c = [c initWithName: [self namePreservingCase: YES] value: [self value] parameters: [self parametersPreservingCase: YES]]; @@ -3722,7 +3736,7 @@ static NSCharacterSet *tokenSet = nil; { NSString *v; - v = [headerClass makeQuoted: [params objectForKey: k] always: NO]; + v = [[self class] makeQuoted: [params objectForKey: k] always: NO]; [m appendString: @"; "]; [m appendString: k]; [m appendString: @"="]; @@ -3769,7 +3783,7 @@ static NSCharacterSet *tokenSet = nil; value: (NSString*)v parameters: (NSDictionary*)p { - n = [headerClass makeToken: n preservingCase: YES]; + n = [[self class] makeToken: n preservingCase: YES]; if ([n length] == 0) { n = @"unknown"; @@ -3807,7 +3821,7 @@ static NSCharacterSet *tokenSet = nil; { return YES; } - if (NO == [other isKindOfClass: headerClass]) + if (NO == [other isKindOfClass: [self class]]) { return NO; } @@ -3876,7 +3890,7 @@ static NSCharacterSet *tokenSet = nil; if (p == nil) { - k = [headerClass makeToken: k]; + k = [[self class] makeToken: k]; p = [params objectForKey: k]; } return p; @@ -4540,7 +4554,7 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, NSUInteger kLength; NSUInteger vLength; - v = [headerClass makeQuoted: [params objectForKey: k] always: NO]; + v = [[self class] makeQuoted: [params objectForKey: k] always: NO]; if (preserve == NO) { k = [k lowercaseString]; @@ -4628,7 +4642,7 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, */ - (void) setParameter: (NSString*)v forKey: (NSString*)k { - k = [headerClass makeToken: k preservingCase: YES]; + k = [[self class] makeToken: k preservingCase: YES]; if (v == nil) { [params removeObjectForKey: k]; @@ -4662,7 +4676,7 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, { NSString *v = [d objectForKey: k]; - k = [headerClass makeToken: k preservingCase: YES]; + k = [[self class] makeToken: k preservingCase: YES]; [m setObject: v forKey: k]; } } @@ -5216,32 +5230,11 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, { if (self == [GSMimeDocument class]) { - NSMutableCharacterSet *m = [[NSMutableCharacterSet alloc] init]; - if (documentClass == 0) { documentClass = [GSMimeDocument class]; } - [m formUnionWithCharacterSet: - [NSCharacterSet characterSetWithCharactersInString: - @".()<>@,;:[]\"\\"]]; - [m formUnionWithCharacterSet: - [NSCharacterSet whitespaceAndNewlineCharacterSet]]; - [m formUnionWithCharacterSet: - [NSCharacterSet controlCharacterSet]]; - [m formUnionWithCharacterSet: - [NSCharacterSet illegalCharacterSet]]; - rfc822Specials = [m copy]; - [[NSObject leakAt: &rfc822Specials] release]; - [m formUnionWithCharacterSet: - [NSCharacterSet characterSetWithCharactersInString: - @"/?="]]; - [m removeCharactersInString: @"."]; - rfc2045Specials = [m copy]; - [[NSObject leakAt: &rfc2045Specials] release]; - [m release]; - whitespace = RETAIN([NSCharacterSet whitespaceAndNewlineCharacterSet]); - [[NSObject leakAt: &whitespace] release]; + if (NSArrayClass == 0) { NSArrayClass = [NSArray class]; @@ -5812,10 +5805,6 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, (void*)@"gb18030"); #endif } - if (headerClass == 0) - { - headerClass = [GSMimeHeader class]; - } } } @@ -5926,7 +5915,7 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, { GSMimeHeader *hdr; - hdr = [headerClass alloc]; + hdr = [GSMimeHeader alloc]; hdr = [hdr initWithName: name value: value parameters: parameters]; @@ -6685,7 +6674,7 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, oaiIMP imp1; boolIMP imp2; - name = [headerClass makeToken: name preservingCase: NO]; + name = [GSMimeHeader makeToken: name preservingCase: NO]; imp1 = (oaiIMP)[headers methodForSelector: @selector(objectAtIndex:)]; imp2 = (boolIMP)[name methodForSelector: @selector(isEqualToString:)]; for (index = 0; index < count; index++) @@ -6710,7 +6699,7 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, { NSUInteger count; - name = [headerClass makeToken: name preservingCase: NO]; + name = [GSMimeHeader makeToken: name preservingCase: NO]; count = [headers count]; if (count > 0) { @@ -6841,9 +6830,9 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, { GSMimeHeader *hdr; - hdr = [[headerClass alloc] initWithName: name - value: value - parameters: parameters]; + hdr = [[GSMimeHeader alloc] initWithName: name + value: value + parameters: parameters]; [self setHeader: hdr]; RELEASE(hdr); return hdr; @@ -7504,9 +7493,9 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, GSMimeParser *p = AUTORELEASE([GSMimeParser new]); NSScanner *scanner = [NSScanner scannerWithString: type]; - hdr = [headerClass headerWithName: @"Content-Type" - value: nil - parameters: nil]; + hdr = [GSMimeHeader headerWithName: @"Content-Type" + value: nil + parameters: nil]; if ([p scanHeaderBody: scanner into: hdr] == NO) { [NSException raise: NSInvalidArgumentException @@ -7519,7 +7508,7 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, NSString *val; val = [NSStringClass stringWithFormat: @"%@/%@", type, subtype]; - hdr = [headerClass alloc]; + hdr = [GSMimeHeader alloc]; hdr = [hdr initWithName: @"Content-Type" value: val parameters: nil]; [hdr setObject: type forKey: @"Type"]; [hdr setObject: subtype forKey: @"Subtype"]; @@ -7565,9 +7554,9 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, p = AUTORELEASE([GSMimeParser new]); scanner = [NSScanner scannerWithString: newType]; - hdr = [headerClass headerWithName: @"Content-Type" - value: nil - parameters: nil]; + hdr = [GSMimeHeader headerWithName: @"Content-Type" + value: nil + parameters: nil]; if ([p scanHeaderBody: scanner into: hdr] == NO) { [NSException raise: NSInvalidArgumentException @@ -7600,7 +7589,7 @@ appendString(NSMutableData *m, NSUInteger offset, NSUInteger fold, { GSMimeHeader *hdr; - hdr = [headerClass alloc]; + hdr = [GSMimeHeader alloc]; hdr = [hdr initWithName: name value: value parameters: parameters]; diff --git a/Source/NSBundle.m b/Source/NSBundle.m index 5132f49d5..37fb8ec2e 100644 --- a/Source/NSBundle.m +++ b/Source/NSBundle.m @@ -840,8 +840,11 @@ _find_paths(NSString *rootPath, NSString *subPath, NSString *localization) if (localization) { - primary = [primary stringByAppendingPathComponent: - [localization stringByAppendingPathExtension: @"lproj"]]; + if ([localization length]) + { + primary = [primary stringByAppendingPathComponent: + [localization stringByAppendingPathExtension: @"lproj"]]; + } contents = bundle_directory_readable(primary); addBundlePath(array, contents, primary, nil, nil); } @@ -861,8 +864,11 @@ _find_paths(NSString *rootPath, NSString *subPath, NSString *localization) } if (localization) { - primary = [originalPrimary stringByAppendingPathComponent: - [localization stringByAppendingPathExtension: @"lproj"]]; + if ([localization length]) + { + primary = [originalPrimary stringByAppendingPathComponent: + [localization stringByAppendingPathExtension: @"lproj"]]; + } contents = bundle_directory_readable(primary); addBundlePath(array, contents, primary, nil, nil); }