Retain count fixes

ChangeLog

@@ -1,3 +1,13 @@
+2024-11-11 Richard Frith-Macdonald <rfm@gnu.org>
+
+	* Source/NSConcreteHashTable.m:
+	* Source/NSConcreteMapTable.m:
+	* Source/NSConcretePointerFunctions.h:
+	* Tests/base/NSHashTable/general.m:
+	* Tests/base/NSMapTable/general.m:
+	Fix for strong memory hash/map table retain count with some simple
+	testcases. Arising from Larry Campbell's comments.
+
 2024-11-09 Richard Frith-Macdonald <rfm@gnu.org>
 
 	* Source/NSNotificationCenter.m:
@@ -10,7 +20,7 @@
 	* Headers/Foundation/NSFileHandle.h:
 	* Source/GSFileHandle.m:
 	* Source/NSFileHandle.m:
-	Add OSX 10.15 file offset methods (issue 325, pull 446 and more)
+	Add OSX 10.15 file offset methods (issue #325, pull #446 and more)
 
 2024-11-03 Richard Frith-Macdonald <rfm@gnu.org>
 

Source/NSConcreteHashTable.m

@@ -87,18 +87,16 @@ @interface	NSConcreteHashTable : NSHashTable
  (M->legacy ? M->cb.old.release(M, X.ptr) \
   : IS_WEAK(M) ? nil : pointerFunctionsRelinquish(&M->cb.pf, &X.ptr))
 #define GSI_MAP_RETAIN_KEY(M, X)\
- (M->legacy ? M->cb.old.retain(M, X.ptr) \
-  : IS_WEAK(M) ? nil : pointerFunctionsAssign(\
-    &M->cb.pf, &X.ptr, pointerFunctionsAcquire(&M->cb.pf, X.ptr)))
+ (M->legacy ? M->cb.old.retain(M, X.ptr) : (IS_WEAK(M) ? nil : X.ptr))
 #define GSI_MAP_ZEROED(M)\
  (M->legacy ? 0 \
  : (IS_WEAK(M) ? YES : NO))
 
 #define GSI_MAP_WRITE_KEY(M, addr, x) \
-	if (M->legacy) \
-		*(addr) = x;\
-	else\
-	 pointerFunctionsAssign(&M->cb.pf, (void**)addr, (x).obj);
+  if (M->legacy) \
+    *(addr) = x;\
+  else\
+    pointerFunctionsReplace(&M->cb.pf, (void**)addr, (x).obj);
 #define GSI_MAP_READ_KEY(M,addr) \
 	(M->legacy ? *(addr) :\
 	 (__typeof__(*addr))pointerFunctionsRead(&M->cb.pf, (void**)addr))

Source/NSConcreteMapTable.m

@@ -94,16 +94,12 @@ @interface	NSConcreteMapTable : NSMapTable
  (M->legacy ? M->cb.old.k.release(M, X.ptr) \
   : IS_WEAK_KEY(M) ? nil : pointerFunctionsRelinquish(&M->cb.pf.k, &X.ptr))
 #define GSI_MAP_RETAIN_KEY(M, X)\
- (M->legacy ? M->cb.old.k.retain(M, X.ptr) \
-  : IS_WEAK_KEY(M) ? nil : pointerFunctionsAssign(&M->cb.pf.k, &X.ptr,\
-    pointerFunctionsAcquire(&M->cb.pf.k, X.ptr)))
+ (M->legacy ? M->cb.old.k.retain(M, X.ptr) : (IS_WEAK_KEY(M) ? nil : X.ptr))
 #define GSI_MAP_RELEASE_VAL(M, X)\
  (M->legacy ? M->cb.old.v.release(M, X.ptr) \
   : IS_WEAK_VALUE(M) ? nil : pointerFunctionsRelinquish(&M->cb.pf.v, &X.ptr))
 #define GSI_MAP_RETAIN_VAL(M, X)\
- (M->legacy ? M->cb.old.v.retain(M, X.ptr) \
-  : IS_WEAK_VALUE(M) ? nil : pointerFunctionsAssign(&M->cb.pf.v, &X.ptr,\
-    pointerFunctionsAcquire(&M->cb.pf.v, X.ptr)))
+ (M->legacy ? M->cb.old.v.retain(M, X.ptr) : (IS_WEAK_VALUE(M) ? nil : &X.ptr))
 
 /* The GSI_MAP_WRITE_KEY() and GSI_MAP_WRITE_VAL() macros are expectd to
  * write without retain (GSI_MAP RETAIN macros are executed separately)
@@ -114,14 +110,12 @@ @interface	NSConcreteMapTable : NSMapTable
   if (M->legacy) \
     *(addr) = x;\
   else\
-    (IS_WEAK_KEY(M) ? pointerFunctionsAssign(&M->cb.pf.k, (void**)addr,\
-      (x).obj) : (*(id*)(addr) = (x).obj));
+    pointerFunctionsReplace(&M->cb.pf.k, (void**)addr, (x).obj);
 #define GSI_MAP_WRITE_VAL(M, addr, x) \
   if (M->legacy) \
     *(addr) = x;\
   else\
-    (IS_WEAK_VALUE(M) ? pointerFunctionsAssign(&M->cb.pf.v, (void**)addr,\
-      (x).obj) : (*(id*)(addr) = (x).obj));
+    pointerFunctionsReplace(&M->cb.pf.v, (void**)addr, (x).obj);
 #define GSI_MAP_READ_KEY(M,addr) \
   (M->legacy ? *(addr)\
     : (__typeof__(*addr))pointerFunctionsRead(&M->cb.pf.k, (void**)addr))

Source/NSConcretePointerFunctions.h

@@ -187,6 +187,8 @@ pointerFunctionsRelinquish(PFInfo *PF, void **itemptr)
     (*PF->relinquishFunction)(*itemptr, PF->sizeFunction);
   if (memoryType(PF->options, NSPointerFunctionsWeakMemory))
     WEAK_WRITE(itemptr, 0);
+  else if (memoryType(PF->options, NSPointerFunctionsStrongMemory))
+    STRONG_WRITE(itemptr, 0);
   else
     *itemptr = 0;
 }
@@ -203,7 +205,7 @@ pointerFunctionsReplace(PFInfo *PF, void **dst, void *src)
       if (PF->relinquishFunction != 0)
 	(*PF->relinquishFunction)(*dst, PF->sizeFunction);
       if (memoryType(PF->options, NSPointerFunctionsWeakMemory))
-        WEAK_WRITE(dst, 0);
+        WEAK_WRITE(dst, src);
       else
 	*dst = src;
     }

Tests/base/NSHashTable/general.m

@@ -1,9 +1,49 @@
 #import <Foundation/NSAutoreleasePool.h>
+#import <Foundation/NSHashTable.h>
 #import "ObjectTesting.h"
 
+@interface MyClass: NSObject
+@end
+
+@implementation	MyClass
+#if 0
+- (oneway void) release
+{
+  NSLog(@"releasing %u", (unsigned)[self retainCount]);
+  [super release];
+}
+- (id) retain
+{
+  id	result = [super retain];
+
+  NSLog(@"retained %u", (unsigned)[self retainCount]);
+  return result;
+}
+#endif
+@end
+
 int main()
 {
-  NSAutoreleasePool   *arp = [NSAutoreleasePool new];
+  NSAutoreleasePool	*arp = [NSAutoreleasePool new];
+  NSHashTable		*t;
+  MyClass		*o;
+  int			c;
+
+  t = [[NSHashTable alloc] initWithOptions: NSHashTableObjectPointerPersonality
+				  capacity: 10];
+
+  o = [MyClass new];
+  c = [o retainCount];
+  PASS(c == 1, "initial retain count is one")
+
+  [t addObject: @"a"];
+  [t addObject: o];
+  PASS([o retainCount] == c + 1, "add to hash table increments retain count")
+
+//  PASS(NSHashGet(t, o) == o, "object found in table")
+
+  [t removeObject: o];
+  PASS([o retainCount] == c, "remove from hash table decrements retain count")
 
   [arp release]; arp = nil;
   return 0;

Tests/base/NSMapTable/general.m

@@ -1,9 +1,55 @@
 #import <Foundation/NSAutoreleasePool.h>
 #import "ObjectTesting.h"
+#import <Foundation/NSMapTable.h>
+
+@interface MyClass: NSObject
+@end
+
+@implementation	MyClass
+#if 1
+- (oneway void) release
+{
+  NSLog(@"releasing %u", (unsigned)[self retainCount]);
+  [super release];
+}
+- (id) retain
+{
+  id	result = [super retain];
+
+  NSLog(@"retained %u", (unsigned)[self retainCount]);
+  return result;
+}
+#endif
+@end
 
 int main()
 {
-  NSAutoreleasePool   *arp = [NSAutoreleasePool new];
+  NSAutoreleasePool	*arp = [NSAutoreleasePool new];
+  NSMapTable		*t;
+  MyClass		*o;
+  int			c;
+
+  t = [[NSMapTable alloc] initWithKeyOptions: NSMapTableObjectPointerPersonality
+				valueOptions: NSMapTableObjectPointerPersonality
+				    capacity: 10];
+
+  o = [MyClass new];
+  c = [o retainCount];
+  PASS(c == 1, "initial retain count is one")
+
+  [t setObject: @"a" forKey: o];
+  PASS([o retainCount] == c + 1, "add map table key increments retain count")
+
+//  PASS(NSHashGet(t, o) == o, "object found in table")
+
+  [t removeObjectForKey: o];
+  PASS([o retainCount] == c, "remove map table key decrements retain count")
+
+  [t setObject: o forKey: @"a"];
+  PASS([o retainCount] == c + 1, "add map table val increments retain count")
+
+  [t removeObjectForKey: @"a"];
+  PASS([o retainCount] == c, "remove map table val decrements retain count")
 
   [arp release]; arp = nil;
   return 0;