diff --git a/ChangeLog b/ChangeLog index 362cbe4dfd..c93137e3d8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,27 @@ +2024-12-27 Richard Frith-Macdonald + + * Headers/GNUstepBase/GSIMap.h: + * Headers/GNUstepBase/GSObjCRuntime.h: + * Source/GSPrivate.h: + * Source/NSConcreteHashTable.m: + * Source/NSConcretePointerFunctions.h: + * Source/NSNotificationCenter.m: + * Source/NSObject.m: + * Source/ObjectiveC2/GNUmakefile: + * Source/ObjectiveC2/README: + * Source/ObjectiveC2/weak.m: + * Tests/base/Functions/runtime.m: + * Tests/base/NSHashTable/weakObjects.m: + * Tests/base/NSMapTable/weakObjects.m: + * Tests/base/NSNotification/general.m: + * Tests/base/NSPointerArray/weakObjects.m: + Implement weak objects API support for classic/gnu runtime. + Use it to implement weak reference support in NSPointerFunctions + (for NSMapTable, NSHashTable, NSP{ointerArray) and to implement + safe observer support for NSNotificationCenter (if an observer is + deallocated then it is removed from the notification center the + next time a notification woudl be posted to it). + 2024-11-29 Richard Frith-Macdonald * Source/NSURLProtocol.m: fix leaks due to retain loops. diff --git a/Headers/GNUstepBase/GSObjCRuntime.h b/Headers/GNUstepBase/GSObjCRuntime.h index 1d8f4ee686..7443a4bc28 100644 --- a/Headers/GNUstepBase/GSObjCRuntime.h +++ b/Headers/GNUstepBase/GSObjCRuntime.h @@ -63,6 +63,18 @@ #endif #endif +#if defined(OBJC_CAP_ARC) +# include +#else +GS_EXPORT void objc_copyWeak(id *dest, id *src); +GS_EXPORT void objc_destroyWeak(id *obj); +GS_EXPORT id objc_initWeak(id *addr, id obj); +GS_EXPORT id objc_loadWeak(id *object); +GS_EXPORT id objc_loadWeakRetained(id *addr); +GS_EXPORT void objc_moveWeak(id *dest, id *src); +GS_EXPORT id objc_storeWeak(id *addr, id obj); +#endif + /* * Hack for older compiler versions that don't have all defines * needed in objc-api.h diff --git a/Source/NSConcretePointerFunctions.h b/Source/NSConcretePointerFunctions.h index 55a8e99fea..3fe0c01d59 100644 --- a/Source/NSConcretePointerFunctions.h +++ b/Source/NSConcretePointerFunctions.h @@ -28,17 +28,15 @@ # include #endif +#import "GNUstepBase/GSObjCRuntime.h" + +#define WEAK_READ(x) objc_loadWeak((id*)x) +#define WEAK_WRITE(addr, x) objc_storeWeak((id*)addr, (id)x) + #if defined(OBJC_CAP_ARC) -# include -# define WEAK_READ(x) objc_loadWeak((id*)x) -# define WEAK_WRITE(addr, x) objc_storeWeak((id*)addr, (id)x) # define STRONG_WRITE(addr, x) objc_storeStrong((id*)addr, (id)x) # define STRONG_ACQUIRE(x) objc_retain(x) #else -extern id objc_loadWeak(id *object); -extern id objc_storeWeak(id *addr, id obj); -# define WEAK_READ(x) objc_loadWeak((id*)x) -# define WEAK_WRITE(addr, x) objc_storeWeak((id*)addr, (id)x) # define STRONG_WRITE(addr, x) ASSIGN(*((id*)addr), ((id)x)) # define STRONG_ACQUIRE(x) RETAIN(((id)x)) #endif diff --git a/Source/NSNotificationCenter.m b/Source/NSNotificationCenter.m index 3ee8c94117..61f19d2324 100644 --- a/Source/NSNotificationCenter.m +++ b/Source/NSNotificationCenter.m @@ -132,17 +132,24 @@ - (NSDictionary*) userInfo * removed from, or not yet added to any list). The end of a * list is marked by 'next' being set to 'ENDOBS'. * - * This is normally a structure which handles memory management using a fast - * reference count mechanism, but when built with clang for GC, a structure - * can't hold a zeroing weak pointer to an observer so it's implemented as a - * trivial class instead ... and gets managed by the garbage collector. + * The observer is a weak reference to the observing object. + * The receiver is a strong reference to the observing object but + * exists only while we are in the process of posting a notification + * to that object (in which case the value of posting is the number + * of times the observation has been added to arrays for posting). + * + * The posting count records the number of times the Observation has + * been added to arrays for posting notification to observers, it is + * needed to track the situation where multiple threads are posting + * notifications to the same server at the same time. */ typedef struct Obs { - id observer; /* Object to receive message. */ + id observer; /* Reference to object. */ + id receiver; /* Retained object (temporary). */ SEL selector; /* Method selector. */ BOOL owner; /* If we own the observer. */ - int32_t retained; /* Retain count for structure. */ + int32_t posting; /* Retain count for structure. */ struct Obs *next; /* Next item in linked list. */ struct NCTbl *link; /* Pointer back to chunk table */ } Observation; @@ -186,17 +193,19 @@ static inline BOOL doEqual(BOOL shouldHash, NSString* key1, NSString* key2) */ static void listFree(Observation *list); -/* Observations have retain/release counts managed explicitly by fast - * function calls. - */ -static void obsRetain(Observation *o); static void obsFree(Observation *o); +/* Observations have their 'posting' counts managed explicitly by fast + * function calls when thye are added to or removed from arrays. + */ +static void obsPost(Observation *o); +static void obsDone(Observation *o); + #define GSI_ARRAY_TYPES 0 #define GSI_ARRAY_TYPE Observation* -#define GSI_ARRAY_RELEASE(A, X) obsFree(X.ext) -#define GSI_ARRAY_RETAIN(A, X) obsRetain(X.ext) +#define GSI_ARRAY_RELEASE(A, X) obsDone(X.ext) +#define GSI_ARRAY_RETAIN(A, X) obsPost(X.ext) #include "GNUstepBase/GSIArray.h" @@ -217,7 +226,7 @@ static inline BOOL doEqual(BOOL shouldHash, NSString* key1, NSString* key2) /* * An NC table is used to keep track of memory allocated to store * Observation structures. When an Observation is removed from the - * notification center, it's memory is returned to the free list of + * notification center, its memory is returned to the free list of * the chunk table, rather than being released to the general * memory allocation system. This means that, once a large number * of observers have been registered, memory usage will never shrink @@ -291,17 +300,17 @@ static inline BOOL doEqual(BOOL shouldHash, NSString* key1, NSString* key2) obs = t->freeList; t->freeList = (Observation*)obs->link; obs->link = (void*)t; - obs->retained = 0; + obs->posting = 0; obs->next = 0; + obs->receiver = nil; obs->selector = s; - obs->observer = o; + objc_initWeak(&obs->observer, o); /* Instances of GSNotificationObserverClass are owned by the Observation * and must be explicitly released when the observation is removed. */ obs->owner = (GSNotificationObserverClass == object_getClass(o)) ? YES : NO; - return obs; } @@ -343,8 +352,7 @@ static void endNCTable(NCTable *t) TEST_RELEASE(t->_lock); - /* - * Free observations without notification names or numbers. + /* Free observations without notification names or numbers. */ listFree(t->wildcard); @@ -433,24 +441,59 @@ static inline void unlockNCTable(NCTable* t) static void obsFree(Observation *o) { - NSCAssert(o->retained >= 0, NSInternalInconsistencyException); - if (o->retained-- == 0) + NCTable *t; + + o->next = 0; // no longer in any active list + if (o->posting) + { + return; // Defer until posting is done + } + + /* If we own the observer, we must release it as well as destroying + * the weak reference to it. + */ + if (o->owner) { - NCTable *t = o->link; + id obj = objc_loadWeak(&o->observer); - if (o->owner) - { - DESTROY(o->observer); - o->owner = NO; - } + [obj autorelease]; // Release to balance the fact we own it. + o->owner = NO; + } + + objc_destroyWeak(&o->observer); + + /* This observation can now be placed in the free list if there is one. + */ + if ((t = o->link) != 0) + { o->link = (NCTable*)t->freeList; t->freeList = o; } } -static void obsRetain(Observation *o) +static void obsDone(Observation *o) +{ + if (0 == --o->posting) + { + /* Posting to this observer is over, so we null-out the receiver + * and let it be deallocated if nobody else has retained it. + */ + [o->receiver autorelease]; + o->receiver = nil; + + /* If the observation was removed from its linked list, it needs + * to be freed now it's no longer being p[osted to. + */ + if (0 == o->next) + { + obsFree(o); + } + } +} + +static void obsPost(Observation *o) { - o->retained++; + o->posting++; } static void listFree(Observation *list) @@ -477,7 +520,7 @@ static void listFree(Observation *list) { Observation *tmp; - while (list != ENDOBS && list->observer == observer) + while (list != ENDOBS && objc_loadWeak(&list->observer) == observer) { tmp = list->next; list->next = 0; @@ -489,7 +532,7 @@ static void listFree(Observation *list) tmp = list; while (tmp->next != ENDOBS) { - if (tmp->next->observer == observer) + if (objc_loadWeak(&tmp->next->observer) == observer) { Observation *next = tmp->next; @@ -546,14 +589,6 @@ static void listFree(Observation *list) } } -/* purgeCollected() returns a list of observations with any observations for - * a collected observer removed. - * purgeCollectedFromMapNode() does the same thing but also handles cleanup - * of the map node containing the list if necessary. - */ -#define purgeCollected(X) (X) -#define purgeCollectedFromMapNode(X, Y) ((Observation*)Y->value.ext) - @interface GSNotificationBlockOperation : NSOperation { @@ -673,14 +708,6 @@ @implementation NSNotificationCenter static NSNotificationCenter *default_center = nil; -+ (void) atExit -{ - id tmp = default_center; - - default_center = nil; - [tmp release]; -} - + (void) initialize { if (self == [NSNotificationCenter class]) @@ -704,7 +731,6 @@ + (void) initialize */ default_center = [self alloc]; [default_center init]; - [self registerAtExit]; } } @@ -819,7 +845,6 @@ - (void) addObserver: (id)observer * once - in which case, the observer will receive multiple messages when * the notification is posted... odd, but the MacOS-X docs specify this. */ - if (name) { /* @@ -1089,6 +1114,55 @@ - (void) removeObserver: (id)observer [self removeObserver: observer name: nil object: nil]; } +static void +addPost(Observation **head, GSIArray a) +{ + Observation *p = 0; + Observation *o = *head; + Observation *t; + + if (0 == o) + { + return; + } + while (o != ENDOBS) + { + t = o->next; + + if (o->receiver) + { + /* Posting already in progress, so we post to the same receiver. + */ + GSIArrayAddItem(a, (GSIArrayItem)o); + p = o; + } + else if ((o->receiver = objc_loadWeakRetained(&o->observer)) != nil) + { + /* We will start posting using a newly retained object as receiver. + */ + GSIArrayAddItem(a, (GSIArrayItem)o); + p = o; + } + else + { + /* The object has been deallocated ... remove the observation from + * the list. + */ + if (p) + { + p->next = t; + } + else + { + *head = t; + } + o->next = 0; + obsFree(o); + } + o = t; + } +} + /** * Private method to perform the actual posting of a notification. @@ -1099,7 +1173,7 @@ - (void) _postAndRelease: (NSNotification*)notification { Observation *o; unsigned count; - NSString *name = [notification name]; + NSString *name; id object; GSIMapNode n; GSIMapTable m; @@ -1107,52 +1181,42 @@ - (void) _postAndRelease: (NSNotification*)notification GSIArray_t b; GSIArray a = &b; - if (name == nil) + if ((name = [notification name]) == nil) { RELEASE(notification); [NSException raise: NSInvalidArgumentException format: @"Tried to post a notification with no name."]; } + + /* Do the rest in an autorelease pool so that the observers can be + * safely released (when the pool ends) outside our locked regions. + */ + ENTER_POOL + object = [notification object]; - /* - * Lock the table of observations while we traverse it. - * - * The table of observations contains weak pointers which are zeroed when - * the observers get destroyed. So to avoid consistency problems - * we use scanned memory in the array in the case where there are more - * than the 64 observers we allowed room for on the stack. + /* Lock the table of observations while we traverse it. */ GSIArrayInitWithZoneAndStaticCapacity(a, _zone, 64, i); + lockNCTable(TABLE); - /* - * Find all the observers that specified neither NAME nor OBJECT. + /* Find all the observers that specified neither NAME nor OBJECT. */ - for (o = WILDCARD = purgeCollected(WILDCARD); o != ENDOBS; o = o->next) - { - GSIArrayAddItem(a, (GSIArrayItem)o); - } + addPost(&WILDCARD, a); - /* - * Find the observers that specified OBJECT, but didn't specify NAME. + /* Find the observers that specified OBJECT, but didn't specify NAME. */ if (object) { n = GSIMapNodeForSimpleKey(NAMELESS, (GSIMapKey)object); - if (n != 0) + if (n) { - o = purgeCollectedFromMapNode(NAMELESS, n); - while (o != ENDOBS) - { - GSIArrayAddItem(a, (GSIArrayItem)o); - o = o->next; - } + addPost(&(n->value.ext), a); } } - /* - * Find the observers of NAME, except those observers with a non-nil OBJECT + /** Find the observers of NAME, except those observers with a non-nil OBJECT * that doesn't match the notification's OBJECT). */ if (name) @@ -1168,34 +1232,22 @@ - (void) _postAndRelease: (NSNotification*)notification } if (m != 0) { - /* - * First, observers with a matching object. + /* First, observers with a matching object. */ n = GSIMapNodeForSimpleKey(m, (GSIMapKey)object); if (n != 0) { - o = purgeCollectedFromMapNode(m, n); - while (o != ENDOBS) - { - GSIArrayAddItem(a, (GSIArrayItem)o); - o = o->next; - } + addPost(&(n->value.ext), a); } if (object != nil) { - /* - * Now observers with a nil object. + /* Now observers with a nil object. */ n = GSIMapNodeForSimpleKey(m, (GSIMapKey)(id)nil); if (n != 0) { - o = purgeCollectedFromMapNode(m, n); - while (o != ENDOBS) - { - GSIArrayAddItem(a, (GSIArrayItem)o); - o = o->next; - } + addPost(&(n->value.ext), a); } } } @@ -1205,8 +1257,7 @@ - (void) _postAndRelease: (NSNotification*)notification */ unlockNCTable(TABLE); - /* - * Now send all the notifications. + /* Now send all the notifications. */ count = GSIArrayCount(a); while (count-- > 0) @@ -1216,7 +1267,7 @@ - (void) _postAndRelease: (NSNotification*)notification { NS_DURING { - [o->observer performSelector: o->selector + [o->receiver performSelector: o->selector withObject: notification]; } NS_HANDLER @@ -1241,11 +1292,17 @@ - (void) _postAndRelease: (NSNotification*)notification NS_ENDHANDLER } } + + /* Cleanup of the array of observations must be lock protected. + */ lockNCTable(TABLE); GSIArrayEmpty(a); unlockNCTable(TABLE); + /* Release the notification and any objects we autoreleased during posting + */ RELEASE(notification); + LEAVE_POOL } diff --git a/Source/ObjectiveC2/weak.m b/Source/ObjectiveC2/weak.m index 6809fdb541..3a3ffc35f6 100644 --- a/Source/ObjectiveC2/weak.m +++ b/Source/ObjectiveC2/weak.m @@ -8,6 +8,9 @@ #import "../GSPrivate.h" #import "../GSPThread.h" +static Class persistentClasses[1]; +static int persistentClassCount = sizeof(persistentClasses)/sizeof(Class); + /* This function needs to identify objects which should NOT be handled by * weak references. * Nil is a special case which can not be stored as a weak reference because @@ -19,14 +22,43 @@ static inline BOOL isPersistentObject(id obj) { - if (obj == nil) + Class c; + int i; + + if (nil == obj) + { + return YES; + } + + /* If the alignment of the object does not match that needed for a + * pointer (to the class of the object) then the object must be a + * special one of some sort and we assume it's persistent. + */ +#if GS_SIZEOF_VOIDP == 8 + if ((intptr_t)obj & 15) { return YES; } - if (object_getClass(obj) == [NSConstantString class]) +#else + if ((intptr_t)obj & 7) { return YES; } +#endif + + c = object_getClass(obj); + if (class_isMetaClass(c)) + { + return YES; // obj was a class rather than an instance + } + + for (i = 0; i < persistentClassCount; i++) + { + if (persistentClasses[i] == c) + { + return YES; // the object is a persistent instance + } + } return NO; } @@ -70,6 +102,7 @@ { GSIMapInitWithZoneAndCapacity( &weakRefs, NSDefaultMallocZone(), 1024); + persistentClasses[0] = [NXConstantString class]; } GS_MUTEX_UNLOCK(weakLock); } diff --git a/Tests/base/Functions/runtime.m b/Tests/base/Functions/runtime.m index 7e0fabcdd9..456b183eef 100644 --- a/Tests/base/Functions/runtime.m +++ b/Tests/base/Functions/runtime.m @@ -2,6 +2,7 @@ #import #import #import +#import #import #import #import @@ -80,7 +81,6 @@ - (const char *) sel2 main(int argc, char *argv[]) { ENTER_POOL - id obj; Class cls; Class meta; SEL sel; @@ -121,7 +121,6 @@ - (const char *) sel2 PASS(0 == class_getVersion(Nil), "class_getVersion() for Nil is 0"); - obj = AUTORELEASE([NSObject new]); cls = [SubClass1 class]; PASS(c1initialize != 0, "+initialize was called"); @@ -296,6 +295,39 @@ - (const char *) sel2 "NSStringFromSelector() works for existing selector"); LEAVE_POOL + + START_SET("weakref") + Class c = [NSObject class]; + id obj; + id got; + id ref; + int rc; + + ref = nil; + + objc_storeWeak(&ref, nil); + PASS(ref == nil, "nil is stored unchanged") + + objc_storeWeak(&ref, @"hello"); + PASS(ref == @"hello", "literal string is stored unchanged") + + objc_storeWeak(&ref, (id)c); + PASS(ref == (id)c, "a class is stored unchanged") + + obj = [NSObject new]; + objc_storeWeak(&ref, obj); + PASS(ref != obj, "object is stored as weak reference") + rc = [obj retainCount]; + got = objc_loadWeakRetained(&ref); + PASS(got == obj && [obj retainCount] == rc + 1, + "objc_loadWeakRetained() returns original retained") + RELEASE(got); + RELEASE(obj); + got = objc_loadWeakRetained(&ref); + PASS(got == nil, "load of deallocated object returns nil") + + END_SET("weakref") + return 0; } diff --git a/Tests/base/NSNotification/general.m b/Tests/base/NSNotification/general.m index 8ee769e5fd..426ded1222 100644 --- a/Tests/base/NSNotification/general.m +++ b/Tests/base/NSNotification/general.m @@ -8,14 +8,26 @@ #define __has_feature(x) 0 #endif -static int notificationCounter = 0; // Real object @interface RealObject : NSObject +{ + int notificationCounter; +} +- (int) count; +- (void) reset; - (void) test: (NSNotification *)aNotification; @end @implementation RealObject +- (int) count +{ + return notificationCounter; +} +- (void) reset +{ + notificationCounter = 0; +} - (void) test: (NSNotification *)aNotification { notificationCounter++; @@ -100,23 +112,21 @@ - (void) forwardInvocation: (NSInvocation *)anInvocation // Create the real object and its proxy RealObject *real = [[[RealObject alloc] init] autorelease]; - ProxyObject *proxy = - [[[ProxyObject alloc] initWithRealObject: real] autorelease]; + ProxyObject *proxy = [[[ProxyObject alloc] + initWithRealObject: real] autorelease]; // Make the proxy object an observer for the sample notification // NB It is important (for the test) to perform this with a local // autorelease pool. - { - NSAutoreleasePool *pool = [NSAutoreleasePool new]; + ENTER_POOL NSLog(@"Adding proxy observer"); [nc addObserver: proxy selector: @selector(test:) name: @"TestNotification" object: testObject]; [nc postNotification: aNotification]; - PASS(1 == notificationCounter, "notification via proxy works immediately") - [pool release]; - } + PASS(1 == [real count], "notification via proxy works immediately") + LEAVE_POOL // Post the notification // NB It is not important that this code is performed with a @@ -124,11 +134,11 @@ - (void) forwardInvocation: (NSInvocation *)anInvocation { NSAutoreleasePool *pool = [NSAutoreleasePool new]; [nc postNotification: aNotification]; - PASS(2 == notificationCounter, "notification via proxy works after pool") + PASS(2 == [real count], "notification via proxy works after pool") [pool release]; } [nc postNotification: aNotification]; - PASS(3 == notificationCounter, "notification via proxy works repeatedly") + PASS(3 == [real count], "notification via proxy works repeatedly") [nc removeObserver: proxy]; @@ -149,7 +159,7 @@ - (void) forwardInvocation: (NSInvocation *)anInvocation o = AUTORELEASE(RETAIN(o)); c = [o retainCount]; [nc postNotification: aNotification]; - PASS(4 == notificationCounter, "notification via block works immediately") + PASS(4 == [real count], "notification via block works immediately") PASS([o retainCount] == c, "observer retain count unaltered on notification") [nc removeObserver: o]; @@ -159,6 +169,28 @@ - (void) forwardInvocation: (NSInvocation *)anInvocation } #endif + /* Now test the feature whereby a deallocated instance is automatically + * removed as an observer (so we don't post to it and crash). + */ + real = [RealObject new]; + PASS(0 == [real count], "initial count is zero") + ENTER_POOL + NSLog(@"Adding real observer"); + [nc addObserver: real + selector: @selector(test:) + name: @"TestNotification" + object: testObject]; + [nc postNotification: aNotification]; + PASS(1 == [real count], "notification works immediately") + LEAVE_POOL + + ENTER_POOL + PASS([real retainCount] == 1, "retain count ready for dealloc") + DESTROY(real); + PASS_RUNS([nc postNotification: aNotification];, + "automatic removal of deallocated instance OK") + LEAVE_POOL + [pool release]; return 0; }