Quantcast

[PATCH 0/2] Thread name support (v2)

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 0/2] Thread name support (v2)

Jon TURNEY
Re-heat Yaakov's patch [1] for adding pthread_getname_np and pthread_setname_np
Use the native interface [2] for sending thread names to debugger
 
[1] https://cygwin.com/ml/cygwin-patches/2012-q1/msg00022.html
[2] https://msdn.microsoft.com/en-us/library/xcb2z8hs.aspx

Changes since v1:
- pthread_setname_np() returns ERANGE for names longer than 16 characters to match linux
- code style fixes
- leave setting the name of the profiling thread in libgmon.a as future work

Jon Turney (2):
  Add pthread_getname_np and pthread_setname_np
  Send thread names to debugger

 winsup/cygwin/common.din               |  2 +
 winsup/cygwin/cygthread.cc             |  2 +
 winsup/cygwin/dcrt0.cc                 |  1 +
 winsup/cygwin/exceptions.cc            |  2 +-
 winsup/cygwin/include/cygwin/version.h |  3 +-
 winsup/cygwin/include/pthread.h        |  2 +
 winsup/cygwin/miscfuncs.cc             | 26 +++++++++++++
 winsup/cygwin/miscfuncs.h              |  2 +
 winsup/cygwin/net.cc                   |  1 +
 winsup/cygwin/release/2.6.0            |  1 +
 winsup/cygwin/thread.cc                | 69 +++++++++++++++++++++++++++++++++-
 winsup/cygwin/thread.h                 |  1 +
 winsup/doc/new-features.xml            |  4 ++
 winsup/doc/posix.xml                   |  2 +
 14 files changed, 115 insertions(+), 3 deletions(-)

--
2.8.3

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/2] Add pthread_getname_np and pthread_setname_np

Jon TURNEY
This patch adds pthread_getname_np and pthread_setname_np.

These were added to glibc in 2.12[1] and are also present in some form on
NetBSD and several UNIXes.

The code is based on NetBSD's implementation with changes to better match
Linux behaviour.

Implementation quirks:

* pthread_setname_np with a NULL pointer segfaults (as linux)

* pthread_setname_np returns ERANGE for names longer than 16 characters (as
linux)

* pthread_getname_np with a NULL pointer returns EFAULT (as linux)

* pthread_getname_np with a buffer length of less than 16 returns ERANGE (as
linux)

* pthread_getname_np truncates the thread name to fit the buffer length.
This guarantees success even when the default thread name is longer than 16
characters, but means there is no way to discover the actual length of the
thread name. (Linux always truncates the thread name to 16 characters)

* Changing program_invocation_short_name changes the default thread name (on
linux, it has no effect on the default thread name)

I'll leave it up to you to decide if any of these matter.

This is implemented via class pthread_attr to make it easier to add
pthread_attr_[gs]etname_np (present in NetBSD and some UNIXes) should it
ever be added to Linux (or we decide we want it anyway).

[1] https://sourceware.org/git/?p=glibc.git;a=blob;f=NEWS
---
 winsup/cygwin/common.din               |  2 ++
 winsup/cygwin/include/cygwin/version.h |  3 +-
 winsup/cygwin/include/pthread.h        |  2 ++
 winsup/cygwin/release/2.6.0            |  1 +
 winsup/cygwin/thread.cc                | 64 +++++++++++++++++++++++++++++++++-
 winsup/cygwin/thread.h                 |  1 +
 winsup/doc/new-features.xml            |  4 +++
 winsup/doc/posix.xml                   |  2 ++
 8 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din
index d54b70a..8f7a282 100644
--- a/winsup/cygwin/common.din
+++ b/winsup/cygwin/common.din
@@ -1046,6 +1046,7 @@ pthread_exit SIGFE
 pthread_getattr_np SIGFE
 pthread_getconcurrency SIGFE
 pthread_getcpuclockid SIGFE
+pthread_getname_np SIGFE
 pthread_getschedparam SIGFE
 pthread_getsequence_np SIGFE
 pthread_getspecific SIGFE
@@ -1086,6 +1087,7 @@ pthread_self SIGFE
 pthread_setcancelstate SIGFE
 pthread_setcanceltype SIGFE
 pthread_setconcurrency SIGFE
+pthread_setname_np SIGFE
 pthread_setschedparam SIGFE
 pthread_setschedprio SIGFE
 pthread_setspecific SIGFE
diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
index 2782c32..a1b8a62 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -467,12 +467,13 @@ details. */
        strtoull_l, wcstod_l, wcstof_l, wcstol_l, wcstold_l, wcstoll_l,
        wcstoul_l, wcstoull_l.
   302: Export nl_langinfo_l.
+  303: Export pthread_getname_np, pthread_setname_np.
 
   Note that we forgot to bump the api for ualarm, strtoll, strtoull,
   sigaltstack, sethostname. */
 
 #define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 302
+#define CYGWIN_VERSION_API_MINOR 303
 
 /* There is also a compatibity version number associated with the shared memory
    regions.  It is incremented when incompatible changes are made to the shared
diff --git a/winsup/cygwin/include/pthread.h b/winsup/cygwin/include/pthread.h
index 609eac2..47ee6bd 100644
--- a/winsup/cygwin/include/pthread.h
+++ b/winsup/cygwin/include/pthread.h
@@ -222,6 +222,8 @@ void pthread_testcancel (void);
 
 #if __GNU_VISIBLE
 int pthread_getattr_np (pthread_t, pthread_attr_t *);
+int pthread_getname_np (pthread_t, char *, size_t) __attribute__((nonnull(2)));
+int pthread_setname_np (pthread_t, const char *) __attribute__((nonnull(2)));
 int pthread_sigqueue (pthread_t *, int, const union sigval);
 int pthread_yield (void);
 #endif
diff --git a/winsup/cygwin/release/2.6.0 b/winsup/cygwin/release/2.6.0
index 3f64577..8255450 100644
--- a/winsup/cygwin/release/2.6.0
+++ b/winsup/cygwin/release/2.6.0
@@ -20,6 +20,7 @@ What's new:
 - locale(1) now supports a -i/--input option to fetch the current input
   locale (this is basically equivalent to the current keyboard layout setting).
 
+- New API: pthread_getname_np, pthread_setname_np.
 
 What changed:
 -------------
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index 4414785..fac801b 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -1106,7 +1106,7 @@ pthread::resume ()
 pthread_attr::pthread_attr ():verifyable_object (PTHREAD_ATTR_MAGIC),
 joinable (PTHREAD_CREATE_JOINABLE), contentionscope (PTHREAD_SCOPE_PROCESS),
 inheritsched (PTHREAD_INHERIT_SCHED), stackaddr (NULL), stacksize (0),
-guardsize (wincap.def_guard_page_size ())
+guardsize (wincap.def_guard_page_size ()), name (NULL)
 {
   schedparam.sched_priority = 0;
 }
@@ -2576,6 +2576,68 @@ pthread_getattr_np (pthread_t thread, pthread_attr_t *attr)
   return 0;
 }
 
+#define NAMELEN 16
+
+extern "C" int
+pthread_getname_np (pthread_t thread, char *buf, size_t buflen)
+{
+  char *name;
+
+  if (!pthread::is_good_object (&thread))
+    return ESRCH;
+
+  if (!thread->attr.name)
+    name = program_invocation_short_name;
+  else
+    name = thread->attr.name;
+
+  /* Return ERANGE if the provided buffer is less than NAMELEN.  Truncate and
+     zero-terminate the name to fit in buf.  This means we always return
+     something if the buffer is NAMELEN or larger, but there is no way to tell
+     if we have the whole name. */
+  if (buflen < NAMELEN)
+    return ERANGE;
+
+  int ret = 0;
+  __try
+    {
+      strlcpy (buf, name, buflen);
+    }
+  __except (NO_ERROR)
+    {
+      ret = EFAULT;
+    }
+  __endtry
+
+  return ret;
+}
+
+extern "C" int
+pthread_setname_np (pthread_t thread, const char *name)
+{
+  char *oldname, *cp;
+
+  if (!pthread::is_good_object (&thread))
+    return ESRCH;
+
+  if (strlen (name) > NAMELEN)
+    return ERANGE;
+
+  cp = strdup (name);
+  if (!cp)
+    return ENOMEM;
+
+  oldname = thread->attr.name;
+  thread->attr.name = cp;
+
+  if (oldname)
+    free (oldname);
+
+  return 0;
+}
+
+#undef NAMELEN
+
 /* provided for source level compatability.
    See http://www.opengroup.org/onlinepubs/007908799/xsh/pthread_getconcurrency.html
 */
diff --git a/winsup/cygwin/thread.h b/winsup/cygwin/thread.h
index 5d51913..48fb6fb 100644
--- a/winsup/cygwin/thread.h
+++ b/winsup/cygwin/thread.h
@@ -240,6 +240,7 @@ public:
   void *stackaddr;
   size_t stacksize;
   size_t guardsize;
+  char *name;
 
   pthread_attr ();
   ~pthread_attr ();
diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
index e3c4ded..7e39316 100644
--- a/winsup/doc/new-features.xml
+++ b/winsup/doc/new-features.xml
@@ -64,6 +64,10 @@ Support AzureAD accounts.
 "nobody" account support for WinFSP.
 </para></listitem>
 
+<listitem><para>
+New API: pthread_getname_np, pthread_setname_np.
+</para></listitem>
+
 </itemizedlist>
 
 </sect2>
diff --git a/winsup/doc/posix.xml b/winsup/doc/posix.xml
index 8d86a12..babf115 100644
--- a/winsup/doc/posix.xml
+++ b/winsup/doc/posix.xml
@@ -1337,6 +1337,8 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008).</para>
     pow10l
     ppoll
     pthread_getattr_np
+    pthread_getname_np
+    pthread_setname_np
     pthread_sigqueue
     ptsname_r
     putwc_unlocked
--
2.8.3

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/2] Send thread names to debugger

Jon TURNEY
In reply to this post by Jon TURNEY
GDB since commit 24cdb46e [1] can report and use these names.

Add utility function SetThreadName(), which sends a thread name to the
debugger.

Use that:
- to set the default thread name for main thread and newly created pthreads.
- in pthread_setname_np() for user thread names.
- for helper thread names in cygthread::create()
- for helper threads which are created directly with CreateThread.

Note that there can still be anonymous threads, created by system or
injected DLLs.

[1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=24cdb46e9f0a694b4fbc11085e094857f08c0419
---
 winsup/cygwin/cygthread.cc  |  2 ++
 winsup/cygwin/dcrt0.cc      |  1 +
 winsup/cygwin/exceptions.cc |  2 +-
 winsup/cygwin/miscfuncs.cc  | 26 ++++++++++++++++++++++++++
 winsup/cygwin/miscfuncs.h   |  2 ++
 winsup/cygwin/net.cc        |  1 +
 winsup/cygwin/thread.cc     |  5 +++++
 7 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/cygthread.cc b/winsup/cygwin/cygthread.cc
index b9d706b..4404e4a 100644
--- a/winsup/cygwin/cygthread.cc
+++ b/winsup/cygwin/cygthread.cc
@@ -213,6 +213,8 @@ cygthread::create ()
     this, 0, &id);
       if (!htobe)
  api_fatal ("CreateThread failed for %s - %p<%y>, %E", __name, h, id);
+      else
+ SetThreadName (GetThreadId (htobe), __name);
       thread_printf ("created name '%s', thread %p, id %y", __name, h, id);
 #ifdef DEBUGGING
       terminated = false;
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index 2328411..8ddee0c 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -964,6 +964,7 @@ dll_crt0_1 (void *)
       if (cp > __progname && ascii_strcasematch (cp, ".exe"))
  *cp = '\0';
     }
+  SetThreadName (GetCurrentThreadId (), program_invocation_short_name);
 
   (void) xdr_set_vprintf (&cygxdr_vwarnx);
   cygwin_finished_initializing = true;
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index d65f56e..0f5a890 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1288,7 +1288,7 @@ DWORD WINAPI
 dumpstack_overflow_wrapper (PVOID arg)
 {
   cygwin_exception *exc = (cygwin_exception *) arg;
-
+  SetThreadName (GetCurrentThreadId (), "__dumpstack_overflow");
   exc->dumpstack ();
   return 0;
 }
diff --git a/winsup/cygwin/miscfuncs.cc b/winsup/cygwin/miscfuncs.cc
index d0e4bf7..5a63b26 100644
--- a/winsup/cygwin/miscfuncs.cc
+++ b/winsup/cygwin/miscfuncs.cc
@@ -1110,3 +1110,29 @@ wmemcpy: \n\
  .seh_endproc \n\
 ");
 #endif
+
+/* Signal the thread name to any attached debugger
+
+   (See "How to: Set a Thread Name in Native Code"
+   https://msdn.microsoft.com/en-us/library/xcb2z8hs.aspx) */
+
+#define MS_VC_EXCEPTION 0x406D1388
+
+void
+SetThreadName(DWORD dwThreadID, const char* threadName)
+{
+  if (!IsDebuggerPresent ())
+    return;
+
+  ULONG_PTR info[] =
+    {
+      0x1000,                 /* type, must be 0x1000 */
+      (ULONG_PTR) threadName, /* pointer to threadname */
+      dwThreadID,             /* thread ID (+ flags on x86_64) */
+#ifdef __X86__
+      0,                      /* flags, must be zero */
+#endif
+    };
+
+  RaiseException (MS_VC_EXCEPTION, 0, sizeof (info)/sizeof (ULONG_PTR), (ULONG_PTR *) &info);
+}
diff --git a/winsup/cygwin/miscfuncs.h b/winsup/cygwin/miscfuncs.h
index a885dcf..5087299 100644
--- a/winsup/cygwin/miscfuncs.h
+++ b/winsup/cygwin/miscfuncs.h
@@ -85,4 +85,6 @@ extern "C" HANDLE WINAPI CygwinCreateThread (LPTHREAD_START_ROUTINE thread_func,
      DWORD creation_flags,
      LPDWORD thread_id);
 
+void SetThreadName (DWORD dwThreadID, const char* threadName);
+
 #endif /*_MISCFUNCS_H*/
diff --git a/winsup/cygwin/net.cc b/winsup/cygwin/net.cc
index 52b3d98..e4805d3 100644
--- a/winsup/cygwin/net.cc
+++ b/winsup/cygwin/net.cc
@@ -1819,6 +1819,7 @@ get_adapters_addresses (PIP_ADAPTER_ADDRESSES *pa_ret, ULONG family)
  The OS allocates stacks bottom up, so chances are good that the new
  stack will be located in the lower address area. */
       HANDLE thr = CreateThread (NULL, 0, call_gaa, &param, 0, NULL);
+      SetThreadName (GetThreadId (thr), "__call_gaa");
       if (!thr)
  {
   debug_printf ("CreateThread: %E");
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index fac801b..1205ce3 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -1992,6 +1992,9 @@ pthread::thread_init_wrapper (void *arg)
   _my_tls.sigmask = thread->parent_sigmask;
   thread->set_tls_self_pointer ();
 
+  // Give thread default name
+  SetThreadName (GetCurrentThreadId (), program_invocation_short_name);
+
   thread->mutex.lock ();
 
   // if thread is detached force cleanup on exit
@@ -2630,6 +2633,8 @@ pthread_setname_np (pthread_t thread, const char *name)
   oldname = thread->attr.name;
   thread->attr.name = cp;
 
+  SetThreadName (GetThreadId (thread->win32_obj_id), thread->attr.name);
+
   if (oldname)
     free (oldname);
 
--
2.8.3

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 1/2] Add pthread_getname_np and pthread_setname_np

Corinna Vinschen-2
In reply to this post by Jon TURNEY
Hi Jon,

On Aug 22 19:08, Jon Turney wrote:
> +#define NAMELEN 16

Please use a more descriptive name here, say  THR_NAMEMAX or so, and add
a "for Linux compat" comment.  With this change the patch is ok to apply.


Thanks,
Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] Fix SetThreadName with current gdb

Jon TURNEY
In reply to this post by Jon TURNEY
Wrap SetThreadName()'s call to RaiseException() in __try/__except/__endtry,
so that if the attached debugger doesn't know about MS_VC_EXCEPTION (e.g.
current gdb and probably strace as well) and continues exception processing,
we ignore it, rather than dying due an unhandled exception.

Signed-off-by: Jon Turney <[hidden email]>
---
 winsup/cygwin/miscfuncs.cc | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/miscfuncs.cc b/winsup/cygwin/miscfuncs.cc
index 5a63b26..7a79359 100644
--- a/winsup/cygwin/miscfuncs.cc
+++ b/winsup/cygwin/miscfuncs.cc
@@ -1134,5 +1134,10 @@ SetThreadName(DWORD dwThreadID, const char* threadName)
 #endif
     };
 
-  RaiseException (MS_VC_EXCEPTION, 0, sizeof (info)/sizeof (ULONG_PTR), (ULONG_PTR *) &info);
+  __try {
+    RaiseException (MS_VC_EXCEPTION, 0, sizeof (info)/sizeof (ULONG_PTR), (ULONG_PTR *) &info);
+  }
+  __except (NO_ERROR) {
+  }
+  __endtry
 }
--
2.8.3

Loading...