NSBundle: fix localization lookup races

Author: triplef

ChangeLog

@@ -1,3 +1,14 @@
+2026-03-12 Frederik Seiffert <frederik@algoriddim.com>
+
+	* Source/NSBundle.m:
+	Hold self alive while resolving localized strings and guard
+	_localizations teardown in -dealloc with _localizationsLock to avoid
+	concurrent access races.
+
+	* Tests/base/NSBundle/localization_concurrency.m: New file.
+	Add a multithreaded regression test for concurrent localization
+	lookups while repeatedly creating and releasing NSBundle instances.
+
 2026-03-12 Richard Frith-Macdonald <rfm@gnu.org>
 
 	* Source/GSHTTPURLHandle.m:

Source/NSBundle.m

@@ -2287,7 +2287,14 @@ - (void) dealloc
   TEST_RELEASE(_frameworkVersion);
   TEST_RELEASE(_bundleClasses);
   TEST_RELEASE(_infoDict);
+  /*
+   * Synchronize teardown with -localizedStringForKey:value:table:
+   * to prevent releasing _localizations while another thread reads/writes it.
+   */
+  GS_MUTEX_LOCK(_localizationsLock);
   TEST_RELEASE(_localizations);
+  _localizations = nil;
+  GS_MUTEX_UNLOCK(_localizationsLock);
   [super dealloc];
 }
 
@@ -2864,6 +2871,13 @@ - (NSString *) localizedStringForKey: (NSString *)key
   NSDictionary	*parsedTable = nil;
   BOOL		shouldLoad = NO;
 
+  /*
+   * Keep the bundle alive for the full duration of this lookup.
+   * This avoids races with concurrent release/dealloc on callers
+   * that do not hold a strong reference to the bundle object.
+   */
+  IF_NO_ARC([self retain];)
+
   // Lazily create and populate the per-bundle localization cache.
   GS_MUTEX_LOCK(_localizationsLock);
   if (_localizations == nil)
@@ -2963,6 +2977,7 @@ - (NSString *) localizedStringForKey: (NSString *)key
         newString = @"";
     }
 
+  IF_NO_ARC([self release];)
   return newString;
 }
 

Tests/base/NSBundle/localization_concurrency.m

@@ -0,0 +1,155 @@
+#import "Testing.h"
+#import <Foundation/NSAutoreleasePool.h>
+#import <Foundation/NSBundle.h>
+#import <Foundation/NSCondition.h>
+#import <Foundation/NSDate.h>
+#import <Foundation/NSFileManager.h>
+#import <Foundation/NSPathUtilities.h>
+#import <Foundation/NSThread.h>
+#import <Foundation/NSString.h>
+
+static NSString *bundlePath = nil;
+static NSCondition *condition = nil;
+static int remainingThreads = 0;
+static int invalidResultCount = 0;
+static int localizedResultCount = 0;
+static int fallbackResultCount = 0;
+
+static const int workerCount = 8;
+static const int iterationsPerWorker = 2500;
+
+@interface LocalizationWorker: NSObject
++ (void) run: (id)unused;
+@end
+
+@implementation LocalizationWorker
+
++ (void) run: (id)unused
+{
+  NSAutoreleasePool *arp = [NSAutoreleasePool new];
+  int localInvalid = 0;
+  int localLocalized = 0;
+  int localFallback = 0;
+  int i;
+
+  for (i = 0; i < iterationsPerWorker; i++)
+    {
+      NSBundle *bundle = [[NSBundle alloc] initWithPath: bundlePath];
+      NSString *str;
+
+      if (nil == bundle)
+        {
+          localInvalid++;
+          continue;
+        }
+
+      str = [bundle localizedStringForKey: @"test"
+                                    value: @"test"
+                                    table: nil];
+      if ([str isEqualToString: @"42"])
+        {
+          localLocalized++;
+        }
+      else if ([str isEqualToString: @"test"])
+        {
+          localFallback++;
+        }
+      else
+        {
+          localInvalid++;
+        }
+
+      str = [bundle localizedStringForKey: @"test"
+                                    value: @"test"
+                                    table: @"Localizable.strings"];
+      if ([str isEqualToString: @"42"])
+        {
+          localLocalized++;
+        }
+      else if ([str isEqualToString: @"test"])
+        {
+          localFallback++;
+        }
+      else
+        {
+          localInvalid++;
+        }
+
+      [bundle release];
+    }
+
+  [condition lock];
+  invalidResultCount += localInvalid;
+  localizedResultCount += localLocalized;
+  fallbackResultCount += localFallback;
+  remainingThreads--;
+  [condition signal];
+  [condition unlock];
+
+  [arp release];
+}
+
+@end
+
+int main()
+{
+  NSAutoreleasePool *arp = [NSAutoreleasePool new];
+  NSString *path;
+  NSBundle *bundle;
+  NSDate *timeout;
+  int i;
+
+  START_SET("NSBundle localizedStringForKey concurrency")
+
+    path = [[[[[NSFileManager defaultManager] currentDirectoryPath]
+      stringByStandardizingPath] stringByAppendingPathComponent: @"Resources"]
+      stringByAppendingPathComponent: @"TestBundle.bundle"];
+    bundle = [NSBundle bundleWithPath: path];
+    if (nil == bundle)
+      {
+        SKIP("test bundle not available");
+      }
+
+    bundlePath = [path copy];
+    condition = [NSCondition new];
+    remainingThreads = workerCount;
+    invalidResultCount = 0;
+    localizedResultCount = 0;
+    fallbackResultCount = 0;
+
+    for (i = 0; i < workerCount; i++)
+      {
+        [NSThread detachNewThreadSelector: @selector(run:)
+                                 toTarget: [LocalizationWorker class]
+                               withObject: nil];
+      }
+
+    timeout = [NSDate dateWithTimeIntervalSinceNow: 60.0];
+    [condition lock];
+    while (remainingThreads > 0)
+      {
+        if (NO == [condition waitUntilDate: timeout])
+          {
+            break;
+          }
+      }
+    PASS(remainingThreads == 0, "all worker threads completed");
+    [condition unlock];
+
+    PASS(invalidResultCount == 0,
+      "localizedStringForKey returned only expected values");
+    PASS(localizedResultCount > 0, "localized lookups succeeded");
+    PASS((localizedResultCount + fallbackResultCount)
+      == (workerCount * iterationsPerWorker * 2),
+      "all lookups accounted for");
+
+    [condition release];
+    condition = nil;
+    [bundlePath release];
+    bundlePath = nil;
+
+  END_SET("NSBundle localizedStringForKey concurrency")
+
+  [arp release];
+  return 0;
+}