Fix infinite loop in NSCFDictionary/NSCFSet fast enumeration

ChangeLog

@@ -1,3 +1,19 @@
+2026-07-02  Rupert Daniel <rupert@algoriddim.com>
+	* Tests/CFDictionary/fast_enumeration.m,
+	* Tests/CFSet/fast_enumeration.m: Add tests for fast enumeration of
+	CF-created dictionaries and sets, covering the empty, single-key,
+	multi-batch, and repeated-enumeration cases.
+
+2026-07-02  Rupert Daniel <rupert@algoriddim.com>
+	* Source/NSCFDictionary.m,
+	* Source/NSCFSet.m: Fix -countByEnumeratingWithState:objects:count:,
+	which created a fresh enumerator on every call so fast enumeration
+	never terminated.  Iterate the backing hash table directly, keeping
+	the resume position in the enumeration state.
+	* Source/GSHashTable.c,
+	* Source/GSHashTable.h: Add GSHashTableGetKeysFromCursor(), an
+	allocation-free resumable key iterator used by the above.
+
 2021-09-30  Frederik Seiffert <frederik@algoriddim.com>
 	* Source/CFDate.c: Fix logic in CFGregorianDateIsValid()
 

Source/GSHashTable.c

@@ -490,6 +490,25 @@ GSHashTableGetKeysAndValues (GSHashTableRef table, const void **keys,
     }
 }
 
+CFIndex
+GSHashTableGetKeysFromCursor (GSHashTableRef table, CFIndex *cursor,
+                              const void **keybuf, CFIndex bufSize)
+{
+  GSHashTableBucket *buckets = table->_buckets;
+  CFIndex capacity = table->_capacity;
+  CFIndex idx = *cursor;
+  CFIndex n = 0;
+
+  while (n < bufSize && idx < capacity)
+    {
+      if (buckets[idx].count > 0)
+        keybuf[n++] = buckets[idx].key;
+      ++idx;
+    }
+  *cursor = idx;
+  return n;
+}
+
 const void *
 GSHashTableGetValue (GSHashTableRef table, const void *key)
 {

Source/GSHashTable.h

@@ -114,6 +114,15 @@ GS_PRIVATE void
 GSHashTableGetKeysAndValues (GSHashTableRef table, const void **keys,
                              const void **values);
 
+/* Resumable key iterator.  Writes up to bufSize keys into keybuf starting at
+ * bucket *cursor, advances *cursor past them, and returns the number written
+ * (0 once the table is exhausted).  Reads live buckets in place and allocates
+ * nothing, so it is suitable as a fast-enumeration primitive.
+ */
+GS_PRIVATE CFIndex
+GSHashTableGetKeysFromCursor (GSHashTableRef table, CFIndex *cursor,
+                              const void **keybuf, CFIndex bufSize);
+
 GS_PRIVATE const void *GSHashTableGetValue (GSHashTableRef table,
                                           const void *key);
 

Source/NSCFDictionary.m

@@ -29,6 +29,7 @@
 
 #include "NSCFType.h"
 #include "CoreFoundation/CFDictionary.h"
+#include "GSHashTable.h"
 
 @interface NSCFDictionary : NSMutableDictionary
 NSCFTYPE_VARS
@@ -136,11 +137,18 @@ - (NSUInteger) countByEnumeratingWithState: (NSFastEnumerationState*)state
                                    objects: (id[])stackbuf
                                      count: (NSUInteger)len
 {
-  NSEnumerator *enuM = [self keyEnumerator];
-
-  return [enuM countByEnumeratingWithState: state
-                                   objects: stackbuf
-                                     count: len];
+  /* Walk the buckets across calls, keeping the resume position in extra[0].
+   * Reading keys in place needs no per-call allocation, and returning 0 when
+   * exhausted ends the enumeration. */
+  CFIndex cursor = (state->state == 0) ? 0 : (CFIndex) state->extra[0];
+  CFIndex n = GSHashTableGetKeysFromCursor ((GSHashTableRef) self, &cursor,
+                                            (const void **) stackbuf,
+                                            (CFIndex) len);
+  state->extra[0] = (unsigned long) cursor;
+  state->itemsPtr = stackbuf;
+  state->mutationsPtr = (unsigned long *) self;
+  state->state += n;
+  return n;
 }
 
 - (void) setObject: anObject forKey: (id)aKey

Source/NSCFSet.m

@@ -29,6 +29,7 @@
 
 #include "NSCFType.h"
 #include "CoreFoundation/CFSet.h"
+#include "GSHashTable.h"
 
 @interface NSCFSet : NSMutableSet
 NSCFTYPE_VARS
@@ -108,12 +109,17 @@ - (NSUInteger) countByEnumeratingWithState: (NSFastEnumerationState*)state
                                    objects: (id*)stackbuf
                                      count: (NSUInteger)len
 {
-  // TODO: inefficient
-  NSEnumerator *enuM = [self objectEnumerator];
-
-  return [enuM countByEnumeratingWithState: state
-                                   objects: stackbuf
-                                     count: len];
+  /* Set elements are stored as the hash table's keys; enumerate them in place,
+   * keeping the resume position in extra[0]. */
+  CFIndex cursor = (state->state == 0) ? 0 : (CFIndex) state->extra[0];
+  CFIndex n = GSHashTableGetKeysFromCursor ((GSHashTableRef) self, &cursor,
+                                            (const void **) stackbuf,
+                                            (CFIndex) len);
+  state->extra[0] = (unsigned long) cursor;
+  state->itemsPtr = stackbuf;
+  state->mutationsPtr = (unsigned long *) self;
+  state->state += n;
+  return n;
 }
 
 - (void) addObject: (id)anObject

Tests/CFDictionary/fast_enumeration.m

@@ -0,0 +1,84 @@
+#import <Foundation/NSDictionary.h>
+#import <Foundation/NSNumber.h>
+#include "CoreFoundation/CFDictionary.h"
+#include "../CFTesting.h"
+
+/* Build a dictionary via the CF API so it is backed by NSCFDictionary
+ * (not a gnustep-base dictionary), which is what exercises the bridge's
+ * fast-enumeration path. */
+static CFMutableDictionaryRef makeCFDict(int count)
+{
+  CFMutableDictionaryRef d = CFDictionaryCreateMutable(NULL, 0,
+    &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
+  for (int i = 0; i < count; i++)
+    {
+      id key = [[NSNumber alloc] initWithInt: i];
+      id val = [[NSNumber alloc] initWithInt: i * 10];
+      CFDictionarySetValue(d, (const void *)key, (const void *)val);
+      [key release];
+      [val release];
+    }
+  return d;
+}
+
+static void testFastEnumeration(int count)
+{
+  CFMutableDictionaryRef cfdict = makeCFDict(count);
+  NSDictionary *dict = (NSDictionary *)cfdict;
+
+  int *seen = calloc(count > 0 ? count : 1, sizeof(int));
+  NSUInteger visited = 0, guard = 0, limit = (NSUInteger)count * 4 + 16;
+  Boolean duplicate = false, invalid = false;
+
+  for (NSNumber *key in dict)
+    {
+      int k = [key intValue];
+      if (k < 0 || k >= count)          invalid = true;
+      else if (seen[k]++)               duplicate = true;
+
+      NSNumber *val = [dict objectForKey: key];   /* usable mid-enumeration */
+      if (!val || [val intValue] != k * 10)        invalid = true;
+
+      visited++;
+      if (++guard > limit) break;        /* regression: never terminates */
+    }
+
+  PASS_CF(guard <= limit,
+    "fast enumeration of a %d-key CFDictionary terminates", count);
+  PASS_CF(visited == (NSUInteger)count,
+    "fast enumeration visits every key exactly once (%d keys, visited %lu)",
+    count, (unsigned long)visited);
+  PASS_CF(!duplicate, "fast enumeration does not repeat keys (%d keys)", count);
+  PASS_CF(!invalid,   "fast enumeration yields valid keys/values (%d keys)", count);
+
+  free(seen);
+  CFRelease(cfdict);
+}
+
+/* A fresh for/in must restart cleanly (state->state resets to 0). */
+static void testReEnumeration(void)
+{
+  CFMutableDictionaryRef cfdict = makeCFDict(10);
+  NSDictionary *dict = (NSDictionary *)cfdict;
+  NSUInteger a = 0, b = 0, guard = 0;
+
+  for (id k in dict) { (void)k; if (++guard > 100) break; a++; }
+  guard = 0;
+  for (id k in dict) { (void)k; if (++guard > 100) break; b++; }
+
+  PASS_CF(a == 10 && b == 10,
+    "a CFDictionary can be fast-enumerated repeatedly (%lu, then %lu)",
+    (unsigned long)a, (unsigned long)b);
+
+  CFRelease(cfdict);
+}
+
+int main(void)
+{
+  testFastEnumeration(0);     /* empty: loop body never runs */
+  testFastEnumeration(1);     /* the originally reported case */
+  testFastEnumeration(20);    /* > 16-element batch: cursor resume across calls */
+  testFastEnumeration(500);   /* many batches + hash collisions */
+  testReEnumeration();
+  return 0;
+}

Tests/CFSet/fast_enumeration.m

@@ -0,0 +1,57 @@
+#import <Foundation/NSSet.h>
+#import <Foundation/NSNumber.h>
+#include "CoreFoundation/CFSet.h"
+#include "../CFTesting.h"
+
+/* Build a set via the CF API so it is backed by NSCFSet, exercising the
+ * bridge's fast-enumeration path. */
+static CFMutableSetRef makeCFSet(int count)
+{
+  CFMutableSetRef s = CFSetCreateMutable(NULL, 0, &kCFTypeSetCallBacks);
+  for (int i = 0; i < count; i++)
+    {
+      id v = [[NSNumber alloc] initWithInt: i];
+      CFSetAddValue(s, (const void *)v);
+      [v release];
+    }
+  return s;
+}
+
+static void testFastEnumeration(int count)
+{
+  CFMutableSetRef cfset = makeCFSet(count);
+  NSSet *set = (NSSet *)cfset;
+
+  int *seen = calloc(count > 0 ? count : 1, sizeof(int));
+  NSUInteger visited = 0, guard = 0, limit = (NSUInteger)count * 4 + 16;
+  Boolean duplicate = false, invalid = false;
+
+  for (NSNumber *v in set)
+    {
+      int k = [v intValue];
+      if (k < 0 || k >= count) invalid = true;
+      else if (seen[k]++)      duplicate = true;
+      visited++;
+      if (++guard > limit) break;        /* regression: never terminates */
+    }
+
+  PASS_CF(guard <= limit,
+    "fast enumeration of a %d-element CFSet terminates", count);
+  PASS_CF(visited == (NSUInteger)count,
+    "fast enumeration visits every element once (%d, visited %lu)",
+    count, (unsigned long)visited);
+  PASS_CF(!duplicate, "fast enumeration does not repeat elements (%d)", count);
+  PASS_CF(!invalid,   "fast enumeration yields valid elements (%d)", count);
+
+  free(seen);
+  CFRelease(cfset);
+}
+
+int main(void)
+{
+  testFastEnumeration(0);
+  testFastEnumeration(1);
+  testFastEnumeration(20);
+  testFastEnumeration(500);
+  return 0;
+}