From 012947f09eb42ea219cc98e2bfe7aba5a4bde2e5 Mon Sep 17 00:00:00 2001
From: Hugo Melder <service@hugomelder.com>
Date: Sun, 8 Sep 2024 16:54:01 +0200
Subject: [PATCH] NSOperationQueue: Proper names for worker threads (#437)

* NSOperationQueue: Give Worker Threads a name

* Update Changelog

* Remove empty string test
---
 ChangeLog                        |  7 +++++
 Headers/Foundation/NSOperation.h | 11 ++++++++
 Source/NSOperation.m             | 47 +++++++++++++++++++++++---------
 Tests/base/NSOperation/basic.m   |  2 --
 4 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 90af0da590..fbd41e4301 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2024-13-08: Hugo Melder <hugo@algoriddim.com>
+
+	* Source/NSOperation.m:
+	* Headers/Foundation/NSOperation.h:
+	Give NSOperationQueue worker threads a proper thread
+	name for easy identification.
+
 2024-12-08 Hugo Melder <hugo@algoriddim.com>
 	* Source/NSThread.m:
 	Fix threadPriority and setThreadPriority: on Android.
diff --git a/Headers/Foundation/NSOperation.h b/Headers/Foundation/NSOperation.h
index 8f3c6944a9..ad12755827 100644
--- a/Headers/Foundation/NSOperation.h
+++ b/Headers/Foundation/NSOperation.h
@@ -224,6 +224,17 @@ enum {
    NSOperationQueueDefaultMaxConcurrentOperationCount = -1
 };
 
+/**
+ * An NSOperationQueue manages a number of NSOperation objects, scheduling
+ * them for execution and managing their dependencies.
+ *
+ * Depending on the configuration of the queue, operations may be executed
+ * concurrently or serially.
+ *
+ * Worker threads are named "NSOperationQ_<number>" by default, but
+ * you can set a name for the queue using the -setName: method.
+ * The suffix "_<number>"" is automatically added to the thread name.
+ */
 GS_EXPORT_CLASS
 @interface NSOperationQueue : NSObject
 {
diff --git a/Source/NSOperation.m b/Source/NSOperation.m
index a30876a802..145c8f9b84 100644
--- a/Source/NSOperation.m
+++ b/Source/NSOperation.m
@@ -50,6 +50,7 @@
   NSMutableArray	*waiting; \
   NSMutableArray	*starting; \
   NSString		*name; \
+  NSString		*threadName; \
   BOOL			suspended; \
   NSInteger		executing; \
   NSInteger		threadCount; \
@@ -63,6 +64,7 @@
 #import "Foundation/NSException.h"
 #import "Foundation/NSKeyValueObserving.h"
 #import "Foundation/NSThread.h"
+#import "Foundation/NSValue.h"
 #import "GNUstepBase/NSArray+GNUstepBase.h"
 #import "GSPrivate.h"
 
@@ -614,7 +616,7 @@ - (void) main
 
 @interface	NSOperationQueue (Private)
 - (void) _execute;
-- (void) _thread;
+- (void) _thread: (NSNumber *) threadNumber;
 - (void) observeValueForKeyPath: (NSString *)keyPath
 		       ofObject: (id)object
                          change: (NSDictionary *)change
@@ -831,6 +833,15 @@ - (id) init
       internal->cond = [[NSConditionLock alloc] initWithCondition: 0];
       [internal->cond setName:
         [NSString stringWithFormat: @"cond-for-op-%p", self]];
+      internal->name = [[NSString alloc] initWithFormat: @"NSOperationQueue %p", self];
+
+      /* Ensure that default thread name can be displayed on systems with a
+       * limited thread name length.
+       *
+       * This value is set to internal->name, when altered with -setName:
+       * Worker threads are not renamed during their lifetime.
+       */
+      internal->threadName = @"NSOperationQ";
     }
   return self;
 }
@@ -850,13 +861,9 @@ - (NSString*) name
   NSString	*s;
 
   [internal->lock lock];
-  if (internal->name == nil)
-    {
-      internal->name
-	= [[NSString alloc] initWithFormat: @"NSOperation %p", self];
-    }
-  s = RETAIN(internal->name);
+  s = [internal->name copy];
   [internal->lock unlock];
+
   return AUTORELEASE(s);
 }
 
@@ -902,13 +909,16 @@ - (void) setMaxConcurrentOperationCount: (NSInteger)cnt
 
 - (void) setName: (NSString*)s
 {
-  if (s == nil) s = @"";
+  if (s == nil) return;
+
   [internal->lock lock];
   if (NO == [internal->name isEqual: s])
     {
       [self willChangeValueForKey: @"name"];
       RELEASE(internal->name);
       internal->name = [s copy];
+      // internal->threadName is unretained
+      internal->threadName = internal->name;
       [self didChangeValueForKey: @"name"];
     }
   [internal->lock unlock];
@@ -1000,12 +1010,22 @@ - (void) observeValueForKeyPath: (NSString *)keyPath
   [self _execute];
 }
 
-- (void) _thread
+- (void) _thread: (NSNumber *) threadNumber
 {
+  NSString *tName;
+  NSThread *current;
+
   CREATE_AUTORELEASE_POOL(arp);
 
-  [[[NSThread currentThread] threadDictionary] setObject: self
-                                                  forKey: threadKey];
+  current = [NSThread currentThread];
+
+  [internal->lock lock];
+  tName = [internal->threadName stringByAppendingFormat: @"_%@", threadNumber];
+  [internal->lock unlock];
+
+  [[current threadDictionary] setObject: self forKey: threadKey];
+  [current setName: tName];
+
   for (;;)
     {
       NSOperation	*op;
@@ -1125,9 +1145,10 @@ - (void) _execute
 	      internal->threadCount++;
 	      NS_DURING
 		{
-		  [NSThread detachNewThreadSelector: @selector(_thread)
+      NSNumber *threadNumber = [NSNumber numberWithInteger: internal->threadCount - 1];
+		  [NSThread detachNewThreadSelector: @selector(_thread:)
 					   toTarget: self
-					 withObject: nil];
+					 withObject: threadNumber];
 		}
 	      NS_HANDLER
 		{
diff --git a/Tests/base/NSOperation/basic.m b/Tests/base/NSOperation/basic.m
index 66c8ef1ad3..869d845d71 100644
--- a/Tests/base/NSOperation/basic.m
+++ b/Tests/base/NSOperation/basic.m
@@ -107,8 +107,6 @@ int main()
   PASS(([[obj1 name] length] > 0), "name has a default");
   [obj1 setName: @"mine"];
   PASS(([[obj1 name] isEqual: @"mine"] == YES), "set name OK");
-  [obj1 setName: nil];
-  PASS(([[obj1 name] isEqual: @""]), "setting null name gives empty string");
 
   PASS(([obj1 maxConcurrentOperationCount] == NSOperationQueueDefaultMaxConcurrentOperationCount), "max concurrent set by default");
   [obj1 setMaxConcurrentOperationCount: 1];