[review] Set names of worker threads
Commit Message
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/176
......................................................................
Set names of worker threads
This adds some configury so that gdb can set the names of worker
threads. This makes them show up more nicely when debugging gdb
itself.
gdb/ChangeLog
2019-10-19 Tom Tromey <tom@tromey.com>
* gdbsupport/thread-pool.c (thread_pool::set_thread_count): Set
name of worker thread.
* gdbsupport/common.m4 (GDB_AC_COMMON): Check for
pthread_setname_np.
* configure, config.in: Rebuild.
gdb/gdbserver/ChangeLog
2019-10-19 Tom Tromey <tom@tromey.com>
* configure, config.in: Rebuild.
Change-Id: I60473d65ae9ae14d8c56ddde39684240c16aaf35
---
M gdb/ChangeLog
M gdb/config.in
M gdb/configure
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/config.in
M gdb/gdbserver/configure
M gdb/gdbsupport/common.m4
M gdb/gdbsupport/thread-pool.c
8 files changed, 43 insertions(+), 9 deletions(-)
Comments
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/176
......................................................................
Patch Set 3:
I'll try to take a look at this tomorrow.
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/176
......................................................................
Patch Set 4:
(3 comments)
Some comments below. Nothing major.
| --- gdb/gdbsupport/thread-pool.c
| +++ gdb/gdbsupport/thread-pool.c
| @@ -22,15 +22,24 @@ #include "common-defs.h"
| #if CXX_STD_THREAD
|
| #include "gdbsupport/thread-pool.h"
| #include "gdbsupport/alt-stack.h"
| #include "gdbsupport/block-signals.h"
| #include <algorithm>
|
| +/* On the off chance that we have the pthread library on a Windows
| + host, but std::thread is not using it, avoid calling
| + pthread_setname_np on Windows. */
PS4, Line 31:
Meh. Judging from
<https://stackoverflow.com/questions/19250008/mingw-stdthread-with-
windows-api>, I think this could be solved. But yeah, we really don't
have to bend backwards in this initial patch.
| +#ifndef _WIN32
| +#ifdef HAVE_PTHREAD_SETNAME_NP
| +#include <pthread.h>
| +#endif
| +#endif
PS4, Line 36:
I'd rather this was:
#ifndef _WIN32
# ifdef HAVE_PTHREAD_SETNAME_NP
# define USE_PTHREAD_SETNAME_NP
# endif
#endif
#ifdef USE_PTHREAD_SETNAME_NP
# include <pthread.h>
#endif
so that below we don't duplicate the checks, and we'd have a single
place to edit in the future if the checks evolve. See below...
| +
| namespace gdb
| {
|
| /* The thread pool detach()s its threads, so that the threads will not
| prevent the process from exiting. However, it was discovered that
| if any detached threads were still waiting on a condition variable,
| then the condition variable's destructor would wait for the threads
| to exit -- defeating the purpose.
...
| @@ -57,14 +66,19 @@ thread_pool::set_thread_count (size_t num_threads)
| if (m_thread_count < num_threads)
| {
| /* Ensure that signals used by gdb are blocked in the new
| threads. */
| block_signals blocker;
| for (size_t i = m_thread_count; i < num_threads; ++i)
| {
| std::thread thread (&thread_pool::thread_function, this);
| +#ifndef _WIN32 /* See the comment at the top of the file. */
| +#ifdef HAVE_PTHREAD_SETNAME_NP
PS4, Line 75:
... here, we'd do:
#ifdef USE_PTHREAD_SETNAME_NP
| + pthread_setname_np (thread.native_handle (), "gdb worker");
| +#endif
| +#endif
| thread.detach ();
| }
| }
| /* If the new size is smaller, terminate some existing threads. */
| if (num_threads < m_thread_count)
| {
Tom Tromey has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/176
......................................................................
Patch Set 4:
(1 comment)
| --- gdb/gdbsupport/thread-pool.c
| +++ gdb/gdbsupport/thread-pool.c
| @@ -23,14 +23,23 @@ #if CXX_STD_THREAD
|
| #include "gdbsupport/thread-pool.h"
| #include "gdbsupport/alt-stack.h"
| #include "gdbsupport/block-signals.h"
| #include <algorithm>
|
| +/* On the off chance that we have the pthread library on a Windows
| + host, but std::thread is not using it, avoid calling
| + pthread_setname_np on Windows. */
| +#ifndef _WIN32
| +#ifdef HAVE_PTHREAD_SETNAME_NP
| +#include <pthread.h>
| +#endif
| +#endif
PS4, Line 36:
> I'd rather this was:
...
I made this change.
| +
| namespace gdb
| {
|
| /* The thread pool detach()s its threads, so that the threads will not
| prevent the process from exiting. However, it was discovered that
| if any detached threads were still waiting on a condition variable,
| then the condition variable's destructor would wait for the threads
| to exit -- defeating the purpose.
@@ -1,5 +1,13 @@
2019-10-19 Tom Tromey <tom@tromey.com>
+ * gdbsupport/thread-pool.c (thread_pool::set_thread_count): Set
+ name of worker thread.
+ * gdbsupport/common.m4 (GDB_AC_COMMON): Check for
+ pthread_setname_np.
+ * configure, config.in: Rebuild.
+
+2019-10-19 Tom Tromey <tom@tromey.com>
+
* python/python.c (class gdbpy_gil): New.
(struct gdbpy_event): Add constructor, destructor, operator().
(gdbpy_post_event): Use run_on_main_thread.
@@ -345,6 +345,9 @@
/* Have PTHREAD_PRIO_INHERIT. */
#undef HAVE_PTHREAD_PRIO_INHERIT
+/* Define to 1 if you have the `pthread_setname_np' function. */
+#undef HAVE_PTHREAD_SETNAME_NP
+
/* Define to 1 if you have the `pthread_sigmask' function. */
#undef HAVE_PTHREAD_SIGMASK
@@ -14387,12 +14387,13 @@
# This check must be here, while LIBS includes any necessary
# threading library.
- for ac_func in pthread_sigmask
+ for ac_func in pthread_sigmask pthread_setname_np
do :
- ac_fn_cxx_check_func "$LINENO" "pthread_sigmask" "ac_cv_func_pthread_sigmask"
-if test "x$ac_cv_func_pthread_sigmask" = xyes; then :
+ as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
+ac_fn_cxx_check_func "$LINENO" "$ac_func" "$as_ac_var"
+if eval test \"x\$"$as_ac_var"\" = x"yes"; then :
cat >>confdefs.h <<_ACEOF
-#define HAVE_PTHREAD_SIGMASK 1
+#define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
_ACEOF
fi
@@ -1,5 +1,9 @@
2019-10-19 Tom Tromey <tom@tromey.com>
+ * configure, config.in: Rebuild.
+
+2019-10-19 Tom Tromey <tom@tromey.com>
+
* remote-utils.c (block_unblock_async_io): Use gdb_sigmask.
* linux-low.c (linux_wait_for_event_filtered, linux_async): Use
gdb_sigmask.
@@ -195,6 +195,9 @@
/* Have PTHREAD_PRIO_INHERIT. */
#undef HAVE_PTHREAD_PRIO_INHERIT
+/* Define to 1 if you have the `pthread_setname_np' function. */
+#undef HAVE_PTHREAD_SETNAME_NP
+
/* Define to 1 if you have the `pthread_sigmask' function. */
#undef HAVE_PTHREAD_SIGMASK
@@ -7725,12 +7725,13 @@
# This check must be here, while LIBS includes any necessary
# threading library.
- for ac_func in pthread_sigmask
+ for ac_func in pthread_sigmask pthread_setname_np
do :
- ac_fn_cxx_check_func "$LINENO" "pthread_sigmask" "ac_cv_func_pthread_sigmask"
-if test "x$ac_cv_func_pthread_sigmask" = xyes; then :
+ as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
+ac_fn_cxx_check_func "$LINENO" "$ac_func" "$as_ac_var"
+if eval test \"x\$"$as_ac_var"\" = x"yes"; then :
cat >>confdefs.h <<_ACEOF
-#define HAVE_PTHREAD_SIGMASK 1
+#define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
_ACEOF
fi
@@ -57,7 +57,7 @@
# This check must be here, while LIBS includes any necessary
# threading library.
- AC_CHECK_FUNCS([pthread_sigmask])
+ AC_CHECK_FUNCS([pthread_sigmask pthread_setname_np])
LIBS="$save_LIBS"
CXXFLAGS="$save_CXXFLAGS"
@@ -23,6 +23,15 @@
#include "gdbsupport/block-signals.h"
#include <algorithm>
+/* On the off chance that we have the pthread library on a Windows
+ host, but std::thread is not using it, avoid calling
+ pthread_setname_np on Windows. */
+#ifndef _WIN32
+#ifdef HAVE_PTHREAD_SETNAME_NP
+#include <pthread.h>
+#endif
+#endif
+
namespace gdb
{
@@ -55,6 +64,11 @@
for (size_t i = m_count; i < num_threads; ++i)
{
std::thread thread (&thread_pool::thread_function, this);
+#ifndef _WIN32 /* See the comment at the top of the file. */
+#ifdef HAVE_PTHREAD_SETNAME_NP
+ pthread_setname_np (thread.native_handle (), "gdb worker");
+#endif
+#endif
thread.detach ();
}
}