View Issue Details

IDProjectCategoryView StatusLast Update
0036950FPCRTLpublic2020-05-03 23:41
ReporterBi0T1N Assigned ToSven Barth  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version3.3.1 
Fixed in Version3.3.1 
Summary0036950: [Patch] Implement thread naming for Linux and Android
DescriptionThis implements thread naming for Linux and Android.

Notes:
- It should define THREADNAME_IS_ANSISTRING, I just done know where
- I cannot test on Android but according to https://android.googlesource.com/platform/bionic/+/40eabe2/libc/bionic/pthread_setname_np.cpp it should be supported
Additional InformationIt needs https://bugs.freepascal.org/view.php?id=36940 to be applied first.
TagsNo tags attached.
Fixed in Revision45233
FPCOldBugId
FPCTarget-
Attached Files

Relationships

related to 0036940 closedSven Barth [Patch] Add support for naming threads to TThreadManager 

Activities

Bi0T1N

2020-04-21 23:13

reporter  

01-Implement_ThreadSetName_for_Linux_Android.patch (1,885 bytes)   
diff --git rtl/linux/pthread.inc rtl/linux/pthread.inc
index c327a9c81e..a4e50e9b31 100644
--- rtl/linux/pthread.inc
+++ rtl/linux/pthread.inc
@@ -191,6 +191,7 @@ Type
     function sem_getvalue (__sem:Psem_t; __sval:Plongint):longint;cdecl;external;
 
     function pthread_mutexattr_settype (__attr: Ppthread_mutexattr_t; Kind:Integer): Integer; cdecl;external;
+    function pthread_setname_np(thread: pthread_t; name: PAnsiChar):cint;cdecl;external;
 
 {$else}
 Var
@@ -272,6 +273,7 @@ Var
     sem_getvalue :   function (__sem:Psem_t; __sval:Plongint):longint;cdecl;
 
     pthread_mutexattr_settype : function(__attr: Ppthread_mutexattr_t; Kind:Integer): Integer; cdecl;
+    pthread_setname_np : Function(thread: pthread_t; name: PAnsiChar):cint;cdecl;
 
 
 Var
@@ -363,6 +365,7 @@ begin
   Pointer(sem_post     ) := dlsym(PthreadDLL,'sem_post');
   Pointer(sem_getvalue ) := dlsym(PthreadDLL,'sem_getvalue');
   Pointer(pthread_mutexattr_settype) := dlsym(PthreadDLL,'pthread_mutexattr_settype');
+  Pointer(pthread_setname_np) := dlsym(PthreadDLL,'pthread_setname_np');
 end;
 
 Function UnLoadPthreads : Boolean;
diff --git rtl/unix/cthreads.pp rtl/unix/cthreads.pp
index 8db6646a18..3181d8f62e 100644
--- rtl/unix/cthreads.pp
+++ rtl/unix/cthreads.pp
@@ -488,8 +488,20 @@ Type  PINTRTLEvent = ^TINTRTLEvent;
 
 
   procedure CThreadSetName(threadHandle: TThreadID; const ThreadName: String);
+    var
+      Name: AnsiString;
     begin
+{$if defined(Linux) or defined(Android)}
+      Name := ThreadName;
+      // length restricted to 16 characters including terminating null byte
+      SetLength(Name, 15);
+      if threadHandle = TThreadID(-1) then
+        pthread_setname_np(pthread_self(), @Name[1])
+      else
+        pthread_setname_np(pthread_t(threadHandle), @Name[1]);
+{$else}
       {$Warning ThreadSetName needs to be implemented}
+{$endif}
     end;
 
 

Bi0T1N

2020-04-21 23:14

reporter   ~0122331

Attached two screenshots how it looks in gdb 'info threads' and 'ps -eT'.
gdb_linux.PNG (179,961 bytes)   
gdb_linux.PNG (179,961 bytes)   
ps-eT.PNG (99,309 bytes)   
ps-eT.PNG (99,309 bytes)   

Bi0T1N

2020-04-29 22:53

reporter   ~0122531

Please find attached the updated version of the patch. It also avoids useless string conversions.
01-Implement_Ansi_and_Unicode_ThreadSetName_for_Linux_Android.patch (2,626 bytes)   
diff --git rtl/linux/pthread.inc rtl/linux/pthread.inc
index 50522c9994..ce8fd42f2f 100644
--- rtl/linux/pthread.inc
+++ rtl/linux/pthread.inc
@@ -183,6 +183,7 @@ Type
     function sem_getvalue (__sem:Psem_t; __sval:Plongint):longint;cdecl;external;
 
     function pthread_mutexattr_settype (__attr: Ppthread_mutexattr_t; Kind:Integer): Integer; cdecl;external;
+    function pthread_setname_np(thread: pthread_t; name: PAnsiChar):cint;cdecl;external;
 
 {$else}
 Var
@@ -264,6 +265,7 @@ Var
     sem_getvalue :   function (__sem:Psem_t; __sval:Plongint):longint;cdecl;
 
     pthread_mutexattr_settype : function(__attr: Ppthread_mutexattr_t; Kind:Integer): Integer; cdecl;
+    pthread_setname_np : function(thread: pthread_t; name: PAnsiChar):cint;cdecl;
 
 
 Var
@@ -355,6 +357,7 @@ begin
   Pointer(sem_post     ) := dlsym(PthreadDLL,'sem_post');
   Pointer(sem_getvalue ) := dlsym(PthreadDLL,'sem_getvalue');
   Pointer(pthread_mutexattr_settype) := dlsym(PthreadDLL,'pthread_mutexattr_settype');
+  Pointer(pthread_setname_np) := dlsym(PthreadDLL,'pthread_setname_np');
 end;
 
 Function UnLoadPthreads : Boolean;
diff --git rtl/unix/cthreads.pp rtl/unix/cthreads.pp
index 89c8d75f99..b2d0b8d01d 100644
--- rtl/unix/cthreads.pp
+++ rtl/unix/cthreads.pp
@@ -488,14 +488,42 @@ Type  PINTRTLEvent = ^TINTRTLEvent;
 
 
   procedure CSetThreadDebugNameA(threadHandle: TThreadID; const ThreadName: AnsiString);
+{$if defined(Linux) or defined(Android)}
+    var
+      CuttedName: AnsiString;
+{$endif}
     begin
-      {$Warning SetThreadDebugName needs to be implemented}
+{$if defined(Linux) or defined(Android)}
+      if Assigned(pthread_setname_np) then
+      begin
+        CuttedName := ThreadName;
+        // length restricted to 16 characters including terminating null byte
+        SetLength(CuttedName, 15);
+        if threadHandle = TThreadID(-1) then
+        begin
+          pthread_setname_np(pthread_self(), @CuttedName[1]);
+        end
+        else
+        begin
+          pthread_setname_np(pthread_t(threadHandle), @CuttedName[1]);
+        end;
+      end;
+{$else}
+       {$Warning SetThreadDebugName needs to be implemented}
+{$endif}
     end;
 
 
   procedure CSetThreadDebugNameU(threadHandle: TThreadID; const ThreadName: UnicodeString);
     begin
-      {$Warning SetThreadDebugName needs to be implemented}
+{$if defined(Linux) or defined(Android)}
+      if Assigned(pthread_setname_np) then
+      begin
+        CSetThreadDebugNameA(threadHandle, AnsiString(ThreadName));
+      end;
+{$else}
+       {$Warning SetThreadDebugName needs to be implemented}
+{$endif}
     end;
 
 

Sven Barth

2020-04-30 14:06

manager   ~0122560

You can only check Assigned(pthread_setname_np) if it is indeed a procedure variable which is only the case if dynpthreads is defined. So you need to protect that condition with that define as well (though it's enough to protect only the line with the if, the begin and end can stay as is).

Also it might be better to do CuttedName := Copy(ThreadName, 1, 15); instead of the separate assignment and setting the length (this will only have a real impact if ThreadName is much larger than 15, but better safe than sorry ;) ).

Bi0T1N

2020-04-30 15:06

reporter   ~0122561

According to the comment in the cthreads.pas it should always be the case:


{ we can combine both compile-time linking and dynamic loading, in order to:
    a) solve a problem on some systems with dynamically loading libpthread if
       it's not linked at compile time
    b) still enabling dynamically checking whether or not certain functions
       are available (could also be implemented via weak linking)
}


That's why I didn't put it behind the dynpthreads define as it could be that the function is not available.

Sven Barth

2020-05-01 14:22

manager   ~0122577

The dynpthreads define is not defined for Android as that target does not define Linux, thus the code will fail to compile.

Just look at this example:

program tprocvar;

{$ifdef useprocvar}
var
  SomeProc: procedure;
{$else}
procedure SomeProc;
begin
end;
{$endif}

begin
  if Assigned(SomeProc) then
    Writeln('Blubb');
end.


If you compile with -duseprocvar it will compile without error, but if you compile without it will fail.

And even if it would always be the case that dynpthreads is defined, the fact alone that there is a define should tell you that some time this could be changed.

Bi0T1N

2020-05-01 18:18

reporter   ~0122582

Seems I overlooked that dynpthreads is not defined for Android, thought it is.
Attached the adjusted patch including the use of Copy().
01-Implement_Ansi_and_Unicode_ThreadSetName_for_Linux_Android_ifdef.patch (2,791 bytes)   
diff --git rtl/linux/pthread.inc rtl/linux/pthread.inc
index 50522c9994..ce8fd42f2f 100644
--- rtl/linux/pthread.inc
+++ rtl/linux/pthread.inc
@@ -183,6 +183,7 @@ Type
     function sem_getvalue (__sem:Psem_t; __sval:Plongint):longint;cdecl;external;
 
     function pthread_mutexattr_settype (__attr: Ppthread_mutexattr_t; Kind:Integer): Integer; cdecl;external;
+    function pthread_setname_np(thread: pthread_t; name: PAnsiChar):cint;cdecl;external;
 
 {$else}
 Var
@@ -264,6 +265,7 @@ Var
     sem_getvalue :   function (__sem:Psem_t; __sval:Plongint):longint;cdecl;
 
     pthread_mutexattr_settype : function(__attr: Ppthread_mutexattr_t; Kind:Integer): Integer; cdecl;
+    pthread_setname_np : function(thread: pthread_t; name: PAnsiChar):cint;cdecl;
 
 
 Var
@@ -355,6 +357,7 @@ begin
   Pointer(sem_post     ) := dlsym(PthreadDLL,'sem_post');
   Pointer(sem_getvalue ) := dlsym(PthreadDLL,'sem_getvalue');
   Pointer(pthread_mutexattr_settype) := dlsym(PthreadDLL,'pthread_mutexattr_settype');
+  Pointer(pthread_setname_np) := dlsym(PthreadDLL,'pthread_setname_np');
 end;
 
 Function UnLoadPthreads : Boolean;
diff --git rtl/unix/cthreads.pp rtl/unix/cthreads.pp
index 89c8d75f99..3e32f87ed0 100644
--- rtl/unix/cthreads.pp
+++ rtl/unix/cthreads.pp
@@ -488,14 +488,49 @@ Type  PINTRTLEvent = ^TINTRTLEvent;
 
 
   procedure CSetThreadDebugNameA(threadHandle: TThreadID; const ThreadName: AnsiString);
+{$if defined(Linux) or defined(Android)}
+    var
+      CuttedName: AnsiString;
+{$endif}
     begin
-      {$Warning SetThreadDebugName needs to be implemented}
+{$if defined(Linux) or defined(Android)}
+  {$ifdef dynpthreads}
+      if Assigned(pthread_setname_np) then
+      begin
+  {$endif dynpthreads}
+        // length restricted to 16 characters including terminating null byte
+        CuttedName:=Copy(ThreadName, 1, 15);
+        if threadHandle=TThreadID(-1) then
+        begin
+          pthread_setname_np(pthread_self(), @CuttedName[1]);
+        end
+        else
+        begin
+          pthread_setname_np(pthread_t(threadHandle), @CuttedName[1]);
+        end;
+  {$ifdef dynpthreads}
+      end;
+  {$endif dynpthreads}
+{$else}
+       {$Warning SetThreadDebugName needs to be implemented}
+{$endif}
     end;
 
 
   procedure CSetThreadDebugNameU(threadHandle: TThreadID; const ThreadName: UnicodeString);
     begin
-      {$Warning SetThreadDebugName needs to be implemented}
+{$if defined(Linux) or defined(Android)}
+  {$ifdef dynpthreads}
+      if Assigned(pthread_setname_np) then
+      begin
+  {$endif dynpthreads}
+        CSetThreadDebugNameA(threadHandle, AnsiString(ThreadName));
+  {$ifdef dynpthreads}
+      end;
+  {$endif dynpthreads}
+{$else}
+       {$Warning SetThreadDebugName needs to be implemented}
+{$endif}
     end;
 
 

Sven Barth

2020-05-03 17:10

manager   ~0122617

Thank you for the patch. I've adjusted it slightly (it's only necessary to ifdef the if-condition, the begin...end block can remain).

Please test and close if okay.

Issue History

Date Modified Username Field Change
2020-04-21 23:13 Bi0T1N New Issue
2020-04-21 23:13 Bi0T1N File Added: 01-Implement_ThreadSetName_for_Linux_Android.patch
2020-04-21 23:14 Bi0T1N Note Added: 0122331
2020-04-21 23:14 Bi0T1N File Added: gdb_linux.PNG
2020-04-21 23:14 Bi0T1N File Added: ps-eT.PNG
2020-04-22 10:30 Sven Barth Relationship added related to 0036940
2020-04-29 22:53 Bi0T1N Note Added: 0122531
2020-04-29 22:53 Bi0T1N File Added: 01-Implement_Ansi_and_Unicode_ThreadSetName_for_Linux_Android.patch
2020-04-30 14:06 Sven Barth Note Added: 0122560
2020-04-30 15:06 Bi0T1N Note Added: 0122561
2020-05-01 14:22 Sven Barth Note Added: 0122577
2020-05-01 18:18 Bi0T1N Note Added: 0122582
2020-05-01 18:18 Bi0T1N File Added: 01-Implement_Ansi_and_Unicode_ThreadSetName_for_Linux_Android_ifdef.patch
2020-05-03 17:10 Sven Barth Assigned To => Sven Barth
2020-05-03 17:10 Sven Barth Status new => resolved
2020-05-03 17:10 Sven Barth Resolution open => fixed
2020-05-03 17:10 Sven Barth Fixed in Version => 3.3.1
2020-05-03 17:10 Sven Barth Fixed in Revision => 45233
2020-05-03 17:10 Sven Barth FPCTarget => -
2020-05-03 17:10 Sven Barth Note Added: 0122617
2020-05-03 23:41 Bi0T1N Status resolved => closed