[PATCH v4] gdb/Python: Added ThreadExitedEvent

[PATCH v4] gdb/Python: Added ThreadExitedEvent

[Simon Farre]

v4:
Doc formatting fixed.

v3:
Eli:
Updated docs & NEWS to reflect new changes. Added
a reference from the .ptid attribute of the ThreadExitedEvent
to the ptid attribute of InferiorThread. To do this,
I've added an anchor to that attribute.

Tom:
Tom requested that I should probably just emit the thread object;
I ran into two issues for this, which I could not resolve in this patch;

1 - The Thread Object (the python type) checks it's own validity
by doing a comparison of it's `thread_info* thread` to nullptr. This
means that any access of it's attributes may (probably, since we are
in "async" land) throw Python exceptions because the thread has been
removed from the thread object. Therefore I've decided in v3 of this
patch to just emit most of the same fields that gdb.InferiorThread has, namely
global_num, name, num and ptid (the 3-attribute tuple provided by
gdb.InferiorThread.ptid).

2 - A python user can hold a global reference to an exiting thread. Thus
in order to have a ThreadExit event that can provide attribute access
reliably (both as a global reference, but also inside the thread exit
handler, as we can never guarantee that it's executed _before_ the
thread_info pointer is removed from the gdbpy thread object),
the `thread_info *` thread pointer must not be null. However, this
comes at the cost of gdb.InferiorThread believing it is "valid" - which means,
that if a user holds takes a global reference to that
exiting event thread object, they can some time later do `t.switch()` at which
point GDB will 'explode' so to speak.

v2:
Fixed white space issues and NULL/nullptr stuff,
as requested by Tom Tromey.

v1:
Currently no event is emitted for a thread exit.

This adds this functionality by emitting a new gdb.ThreadExitedEvent.

It currently provides four attributes:
- global_num: The GDB assigned global thread number
- num: the per-inferior thread number
- name: name of the thread or none if not set
- ptid: the PTID of the thread, a 3-attribute tuple, identical to
InferiorThread.ptid attribute

Added info to docs & the NEWS file as well.

Added test to test suite.

Fixed formatting.

Feedback wanted and appreciated.
---
 gdb/NEWS                                      |  3 ++
 gdb/doc/python.texi                           | 22 ++++++++
 gdb/python/py-all-events.def                  |  1 +
 gdb/python/py-event-types.def                 |  5 ++
 gdb/python/py-event.h                         |  4 ++
 gdb/python/py-inferior.c                      |  5 ++
 gdb/python/py-threadevent.c                   | 35 +++++++++++++
 gdb/testsuite/gdb.python/py-thread-exited.c   | 37 ++++++++++++++
 gdb/testsuite/gdb.python/py-thread-exited.exp | 51 +++++++++++++++++++
 gdb/testsuite/gdb.python/py-thread-exited.py  | 31 +++++++++++
 10 files changed, 194 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/py-thread-exited.c
 create mode 100644 gdb/testsuite/gdb.python/py-thread-exited.exp
 create mode 100644 gdb/testsuite/gdb.python/py-thread-exited.py

diff --git a/gdb/NEWS b/gdb/NEWS
index 760cb2b7abc..4231e5181fb 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -38,6 +38,9 @@ maintenance info line-table
      This is the same format that GDB uses when printing address, symbol,
      and offset information from the disassembler.
 
+  ** gdb.ThreadExitedEvent added.  Emits GLOBAL NUM, NAME, PER INFERIOR NUM
+     and PTID of the exiting thread.
+
 *** Changes in GDB 12
 
 * DBX mode is deprecated, and will be removed in GDB 13
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 7c414b01d70..b80904041b2 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3568,6 +3568,27 @@ This has a single attribute:
 The new thread.
 @end defvar
 
+@item events.thread_exited
+This is emitted when @value{GDBN} notices a thread has exited.  The event
+is of type @code{gdb.ThreadExitedEvent}.  This has four attributes:
+
+@defvar ThreadExitedEvent.global_num
+Global thread number.
+@end defvar
+
+@defvar ThreadExitedEvent.name
+The thread's name.  Returns @code{None} if it has none or can't be resolved.
+@end defvar
+
+@defvar ThreadExitedEvent.num
+The thread's per-inferior number.
+@end defvar
+
+@defvar ThreadExitedEvent.ptid
+The thread's ptid
+(@pxref{inferior_thread_ptid,,The InferiorThread.ptid attribute}).
+@end defvar
+
 @item events.gdb_exiting
 This is emitted when @value{GDBN} exits.  This event is not emitted if
 @value{GDBN} exits as a result of an internal error, or after an
@@ -3632,6 +3653,7 @@ make Python breakpoints thread-specific, for example
 (@pxref{python_breakpoint_thread,,The Breakpoint.thread attribute}).
 @end defvar
 
+@anchor{inferior_thread_ptid}
 @defvar InferiorThread.ptid
 ID of the thread, as assigned by the operating system.  This attribute is a
 tuple containing three integers.  The first is the Process ID (PID); the second
diff --git a/gdb/python/py-all-events.def b/gdb/python/py-all-events.def
index 7db8efa1390..0dd1a295135 100644
--- a/gdb/python/py-all-events.def
+++ b/gdb/python/py-all-events.def
@@ -31,6 +31,7 @@ GDB_PY_DEFINE_EVENT(clear_objfiles)
 GDB_PY_DEFINE_EVENT(new_inferior)
 GDB_PY_DEFINE_EVENT(inferior_deleted)
 GDB_PY_DEFINE_EVENT(new_thread)
+GDB_PY_DEFINE_EVENT(thread_exited)
 GDB_PY_DEFINE_EVENT(inferior_call)
 GDB_PY_DEFINE_EVENT(memory_changed)
 GDB_PY_DEFINE_EVENT(register_changed)
diff --git a/gdb/python/py-event-types.def b/gdb/python/py-event-types.def
index 596e68a852a..4529ddc7caa 100644
--- a/gdb/python/py-event-types.def
+++ b/gdb/python/py-event-types.def
@@ -51,6 +51,11 @@ GDB_PY_DEFINE_EVENT_TYPE (new_thread,
 			  "GDB new thread event object",
 			  thread_event_object_type);
 
+GDB_PY_DEFINE_EVENT_TYPE (thread_exited,
+			  "ThreadExitedEvent",
+			  "GDB thread exited event object",
+			  event_object_type);
+
 GDB_PY_DEFINE_EVENT_TYPE (new_inferior,
 			  "NewInferiorEvent",
 			  "GDB new inferior event object",
diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
index 56e53b7a1e7..3619e12df10 100644
--- a/gdb/python/py-event.h
+++ b/gdb/python/py-event.h
@@ -61,6 +61,10 @@ extern int emit_memory_changed_event (CORE_ADDR addr, ssize_t len);
 extern int evpy_emit_event (PyObject *event,
 			    eventregistry_object *registry);
 
+/* Emits a thread exit event for THREAD containing the attributes
+   global_num, name, num and ptid */
+extern int emit_thread_exit_event (thread_info * thread);
+
 extern gdbpy_ref<> create_event_object (PyTypeObject *py_type);
 
 /* thread events can either be thread specific or process wide.  If gdb is
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index ebcd5b0a70f..5f0cd67970e 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -363,6 +363,11 @@ delete_thread_object (struct thread_info *tp, int ignore)
     return;
 
   tmp = *entry;
+  if (!evregpy_no_listeners_p (gdb_py_events.thread_exited))
+    {
+      if (emit_thread_exit_event (tmp->thread_obj->thread) < 0)
+	gdbpy_print_stack ();
+    }
   tmp->thread_obj->thread = NULL;
 
   *entry = (*entry)->next;
diff --git a/gdb/python/py-threadevent.c b/gdb/python/py-threadevent.c
index 0a5d30087fb..305941306af 100644
--- a/gdb/python/py-threadevent.c
+++ b/gdb/python/py-threadevent.c
@@ -54,3 +54,38 @@ create_thread_event_object (PyTypeObject *py_type, PyObject *thread)
 
   return thread_event_obj;
 }
+
+/* Emits a thread exit event for THREAD containing the attributes
+   global_num, name, num and ptid */
+
+int
+emit_thread_exit_event (thread_info * thread)
+{
+  gdbpy_ref<> thread_event_obj
+		= create_event_object (&thread_exited_event_object_type);
+  ptid_t ptid = thread->ptid;
+  int global_num = thread->global_num;
+  const char* name = thread_name (thread);
+  int num = thread->per_inf_num;
+
+  if (thread_event_obj == nullptr)
+    return -1;
+
+  if (evpy_add_attribute (thread_event_obj.get (), "global_num",
+			  PyLong_FromLong (global_num)) < 0)
+    return -1;
+  PyObject * t_name = name == nullptr ? Py_None : PyUnicode_FromString (name);
+
+  if (evpy_add_attribute (thread_event_obj.get (), "name", t_name) < 0)
+	return -1;
+
+  if (evpy_add_attribute (thread_event_obj.get (), "num",
+			  PyLong_FromLong (num)) < 0)
+    return -1;
+
+  if (evpy_add_attribute (thread_event_obj.get (), "ptid",
+			  gdbpy_create_ptid_object (ptid)) < 0)
+    return -1;
+
+  return evpy_emit_event (thread_event_obj.get (), gdb_py_events.thread_exited);
+}
diff --git a/gdb/testsuite/gdb.python/py-thread-exited.c b/gdb/testsuite/gdb.python/py-thread-exited.c
new file mode 100644
index 00000000000..d4ff55b1b44
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-thread-exited.c
@@ -0,0 +1,37 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010-2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see  <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <pthread.h>
+#include <unistd.h>
+#include <signal.h>
+pthread_t thread2_id;
+pthread_t thread3_id;
+
+void* do_thread (void* d)
+{
+  return NULL;
+}
+
+int main (void)
+{
+  pthread_create (&thread2_id, NULL, do_thread, NULL);
+  pthread_join (thread2_id, NULL);
+  pthread_create (&thread3_id, NULL, do_thread, NULL);
+  pthread_join (thread3_id, NULL);
+  return 12;
+}
diff --git a/gdb/testsuite/gdb.python/py-thread-exited.exp b/gdb/testsuite/gdb.python/py-thread-exited.exp
new file mode 100644
index 00000000000..3c66cc5e08d
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-thread-exited.exp
@@ -0,0 +1,51 @@
+# Copyright (C) 2022-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if { ![support_displaced_stepping] } {
+    unsupported "displaced stepping"
+    return -1
+}
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    return -1
+}
+
+save_vars { GDBFLAGS } {
+    clean_restart $testfile
+}
+
+if { [skip_python_tests] } { continue }
+
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/py-thread-exited.py]
+gdb_test_no_output "source ${pyfile}" "load python file"
+
+gdb_test "test-events" "Event testers registered."
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint 37 "last of main"
+
+gdb_continue_to_breakpoint "continue to breakpoint"
+
+gdb_test "python print(threadOneExit)" \
+	 ".*event type: thread-exited. global num: 2.*"
+gdb_test "python print(threadTwoExit)" \
+	 ".*event type: thread-exited. global num: 3.*"
diff --git a/gdb/testsuite/gdb.python/py-thread-exited.py b/gdb/testsuite/gdb.python/py-thread-exited.py
new file mode 100644
index 00000000000..217fcd0ff6b
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-thread-exited.py
@@ -0,0 +1,31 @@
+import gdb
+
+threadOneExit = ""
+threadTwoExit = ""
+# we don't want to overwrite the 2nd thread's exit event, thus
+# store it here. we don't care about it though.
+mainThreadExit = ""
+
+def thread_exited_handler(event):
+    global threadOneExit, threadTwoExit, mainThreadExit
+    print("{}".format(event))
+    assert isinstance(event, gdb.ThreadExitedEvent)
+    if threadOneExit == "":
+        threadOneExit = "event type: thread-exited. global num: {}".format(event.global_num)
+    else:
+        if threadTwoExit == "":
+            threadTwoExit = "event type: thread-exited. global num: {}".format(event.global_num)
+        else:
+            mainThreadExit = "event type: thread-exited. global num: {}".format(event.global_num)
+
+class test_events(gdb.Command):
+    """Test events."""
+
+    def __init__(self):
+        gdb.Command.__init__(self, "test-events", gdb.COMMAND_STACK)
+
+    def invoke(self, arg, from_tty):
+        gdb.events.thread_exited.connect(thread_exited_handler)
+        print("Event testers registered.")
+
+test_events()

base-commit: bd1c798f0aef38493c5292917e47f76e1205f4e3
-- 
2.32.0

[PING][PATCH v4] gdb/Python: Added ThreadExitedEvent

[Simon Farre]

Kindly requesting further review. Docs reviewed prior by Eli.

Thanks!

On Wed, Apr 20, 2022 at 10:25 AM Simon Farre <simon.farre.cx@gmail.com>
wrote:

> v4:
> Doc formatting fixed.
>
> v3:
> Eli:
> Updated docs & NEWS to reflect new changes. Added
> a reference from the .ptid attribute of the ThreadExitedEvent
> to the ptid attribute of InferiorThread. To do this,
> I've added an anchor to that attribute.
>
> Tom:
> Tom requested that I should probably just emit the thread object;
> I ran into two issues for this, which I could not resolve in this patch;
>
> 1 - The Thread Object (the python type) checks it's own validity
> by doing a comparison of it's `thread_info* thread` to nullptr. This
> means that any access of it's attributes may (probably, since we are
> in "async" land) throw Python exceptions because the thread has been
> removed from the thread object. Therefore I've decided in v3 of this
> patch to just emit most of the same fields that gdb.InferiorThread has,
> namely
> global_num, name, num and ptid (the 3-attribute tuple provided by
> gdb.InferiorThread.ptid).
>
> 2 - A python user can hold a global reference to an exiting thread. Thus
> in order to have a ThreadExit event that can provide attribute access
> reliably (both as a global reference, but also inside the thread exit
> handler, as we can never guarantee that it's executed _before_ the
> thread_info pointer is removed from the gdbpy thread object),
> the `thread_info *` thread pointer must not be null. However, this
> comes at the cost of gdb.InferiorThread believing it is "valid" - which
> means,
> that if a user holds takes a global reference to that
> exiting event thread object, they can some time later do `t.switch()` at
> which
> point GDB will 'explode' so to speak.
>
> v2:
> Fixed white space issues and NULL/nullptr stuff,
> as requested by Tom Tromey.
>
> v1:
> Currently no event is emitted for a thread exit.
>
> This adds this functionality by emitting a new gdb.ThreadExitedEvent.
>
> It currently provides four attributes:
> - global_num: The GDB assigned global thread number
> - num: the per-inferior thread number
> - name: name of the thread or none if not set
> - ptid: the PTID of the thread, a 3-attribute tuple, identical to
> InferiorThread.ptid attribute
>
> Added info to docs & the NEWS file as well.
>
> Added test to test suite.
>
> Fixed formatting.
>
> Feedback wanted and appreciated.
> ---
>  gdb/NEWS                                      |  3 ++
>  gdb/doc/python.texi                           | 22 ++++++++
>  gdb/python/py-all-events.def                  |  1 +
>  gdb/python/py-event-types.def                 |  5 ++
>  gdb/python/py-event.h                         |  4 ++
>  gdb/python/py-inferior.c                      |  5 ++
>  gdb/python/py-threadevent.c                   | 35 +++++++++++++
>  gdb/testsuite/gdb.python/py-thread-exited.c   | 37 ++++++++++++++
>  gdb/testsuite/gdb.python/py-thread-exited.exp | 51 +++++++++++++++++++
>  gdb/testsuite/gdb.python/py-thread-exited.py  | 31 +++++++++++
>  10 files changed, 194 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.python/py-thread-exited.c
>  create mode 100644 gdb/testsuite/gdb.python/py-thread-exited.exp
>  create mode 100644 gdb/testsuite/gdb.python/py-thread-exited.py
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 760cb2b7abc..4231e5181fb 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -38,6 +38,9 @@ maintenance info line-table
>       This is the same format that GDB uses when printing address, symbol,
>       and offset information from the disassembler.
>
> +  ** gdb.ThreadExitedEvent added.  Emits GLOBAL NUM, NAME, PER INFERIOR
> NUM
> +     and PTID of the exiting thread.
> +
>  *** Changes in GDB 12
>
>  * DBX mode is deprecated, and will be removed in GDB 13
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 7c414b01d70..b80904041b2 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -3568,6 +3568,27 @@ This has a single attribute:
>  The new thread.
>  @end defvar
>
> +@item events.thread_exited
> +This is emitted when @value{GDBN} notices a thread has exited.  The event
> +is of type @code{gdb.ThreadExitedEvent}.  This has four attributes:
> +
> +@defvar ThreadExitedEvent.global_num
> +Global thread number.
> +@end defvar
> +
> +@defvar ThreadExitedEvent.name
> +The thread's name.  Returns @code{None} if it has none or can't be
> resolved.
> +@end defvar
> +
> +@defvar ThreadExitedEvent.num
> +The thread's per-inferior number.
> +@end defvar
> +
> +@defvar ThreadExitedEvent.ptid
> +The thread's ptid
> +(@pxref{inferior_thread_ptid,,The InferiorThread.ptid attribute}).
> +@end defvar
> +
>  @item events.gdb_exiting
>  This is emitted when @value{GDBN} exits.  This event is not emitted if
>  @value{GDBN} exits as a result of an internal error, or after an
> @@ -3632,6 +3653,7 @@ make Python breakpoints thread-specific, for example
>  (@pxref{python_breakpoint_thread,,The Breakpoint.thread attribute}).
>  @end defvar
>
> +@anchor{inferior_thread_ptid}
>  @defvar InferiorThread.ptid
>  ID of the thread, as assigned by the operating system.  This attribute is
> a
>  tuple containing three integers.  The first is the Process ID (PID); the
> second
> diff --git a/gdb/python/py-all-events.def b/gdb/python/py-all-events.def
> index 7db8efa1390..0dd1a295135 100644
> --- a/gdb/python/py-all-events.def
> +++ b/gdb/python/py-all-events.def
> @@ -31,6 +31,7 @@ GDB_PY_DEFINE_EVENT(clear_objfiles)
>  GDB_PY_DEFINE_EVENT(new_inferior)
>  GDB_PY_DEFINE_EVENT(inferior_deleted)
>  GDB_PY_DEFINE_EVENT(new_thread)
> +GDB_PY_DEFINE_EVENT(thread_exited)
>  GDB_PY_DEFINE_EVENT(inferior_call)
>  GDB_PY_DEFINE_EVENT(memory_changed)
>  GDB_PY_DEFINE_EVENT(register_changed)
> diff --git a/gdb/python/py-event-types.def b/gdb/python/py-event-types.def
> index 596e68a852a..4529ddc7caa 100644
> --- a/gdb/python/py-event-types.def
> +++ b/gdb/python/py-event-types.def
> @@ -51,6 +51,11 @@ GDB_PY_DEFINE_EVENT_TYPE (new_thread,
>                           "GDB new thread event object",
>                           thread_event_object_type);
>
> +GDB_PY_DEFINE_EVENT_TYPE (thread_exited,
> +                         "ThreadExitedEvent",
> +                         "GDB thread exited event object",
> +                         event_object_type);
> +
>  GDB_PY_DEFINE_EVENT_TYPE (new_inferior,
>                           "NewInferiorEvent",
>                           "GDB new inferior event object",
> diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
> index 56e53b7a1e7..3619e12df10 100644
> --- a/gdb/python/py-event.h
> +++ b/gdb/python/py-event.h
> @@ -61,6 +61,10 @@ extern int emit_memory_changed_event (CORE_ADDR addr,
> ssize_t len);
>  extern int evpy_emit_event (PyObject *event,
>                             eventregistry_object *registry);
>
> +/* Emits a thread exit event for THREAD containing the attributes
> +   global_num, name, num and ptid */
> +extern int emit_thread_exit_event (thread_info * thread);
> +
>  extern gdbpy_ref<> create_event_object (PyTypeObject *py_type);
>
>  /* thread events can either be thread specific or process wide.  If gdb is
> diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
> index ebcd5b0a70f..5f0cd67970e 100644
> --- a/gdb/python/py-inferior.c
> +++ b/gdb/python/py-inferior.c
> @@ -363,6 +363,11 @@ delete_thread_object (struct thread_info *tp, int
> ignore)
>      return;
>
>    tmp = *entry;
> +  if (!evregpy_no_listeners_p (gdb_py_events.thread_exited))
> +    {
> +      if (emit_thread_exit_event (tmp->thread_obj->thread) < 0)
> +       gdbpy_print_stack ();
> +    }
>    tmp->thread_obj->thread = NULL;
>
>    *entry = (*entry)->next;
> diff --git a/gdb/python/py-threadevent.c b/gdb/python/py-threadevent.c
> index 0a5d30087fb..305941306af 100644
> --- a/gdb/python/py-threadevent.c
> +++ b/gdb/python/py-threadevent.c
> @@ -54,3 +54,38 @@ create_thread_event_object (PyTypeObject *py_type,
> PyObject *thread)
>
>    return thread_event_obj;
>  }
> +
> +/* Emits a thread exit event for THREAD containing the attributes
> +   global_num, name, num and ptid */
> +
> +int
> +emit_thread_exit_event (thread_info * thread)
> +{
> +  gdbpy_ref<> thread_event_obj
> +               = create_event_object (&thread_exited_event_object_type);
> +  ptid_t ptid = thread->ptid;
> +  int global_num = thread->global_num;
> +  const char* name = thread_name (thread);
> +  int num = thread->per_inf_num;
> +
> +  if (thread_event_obj == nullptr)
> +    return -1;
> +
> +  if (evpy_add_attribute (thread_event_obj.get (), "global_num",
> +                         PyLong_FromLong (global_num)) < 0)
> +    return -1;
> +  PyObject * t_name = name == nullptr ? Py_None : PyUnicode_FromString
> (name);
> +
> +  if (evpy_add_attribute (thread_event_obj.get (), "name", t_name) < 0)
> +       return -1;
> +
> +  if (evpy_add_attribute (thread_event_obj.get (), "num",
> +                         PyLong_FromLong (num)) < 0)
> +    return -1;
> +
> +  if (evpy_add_attribute (thread_event_obj.get (), "ptid",
> +                         gdbpy_create_ptid_object (ptid)) < 0)
> +    return -1;
> +
> +  return evpy_emit_event (thread_event_obj.get (),
> gdb_py_events.thread_exited);
> +}
> diff --git a/gdb/testsuite/gdb.python/py-thread-exited.c
> b/gdb/testsuite/gdb.python/py-thread-exited.c
> new file mode 100644
> index 00000000000..d4ff55b1b44
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-thread-exited.c
> @@ -0,0 +1,37 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2010-2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see  <http://www.gnu.org/licenses/>.
> */
> +
> +#include <stdio.h>
> +#include <pthread.h>
> +#include <unistd.h>
> +#include <signal.h>
> +pthread_t thread2_id;
> +pthread_t thread3_id;
> +
> +void* do_thread (void* d)
> +{
> +  return NULL;
> +}
> +
> +int main (void)
> +{
> +  pthread_create (&thread2_id, NULL, do_thread, NULL);
> +  pthread_join (thread2_id, NULL);
> +  pthread_create (&thread3_id, NULL, do_thread, NULL);
> +  pthread_join (thread3_id, NULL);
> +  return 12;
> +}
> diff --git a/gdb/testsuite/gdb.python/py-thread-exited.exp
> b/gdb/testsuite/gdb.python/py-thread-exited.exp
> new file mode 100644
> index 00000000000..3c66cc5e08d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-thread-exited.exp
> @@ -0,0 +1,51 @@
> +# Copyright (C) 2022-2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +if { ![support_displaced_stepping] } {
> +    unsupported "displaced stepping"
> +    return -1
> +}
> +
> +load_lib gdb-python.exp
> +
> +standard_testfile
> +
> +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}"
> executable {debug}] != "" } {
> +    return -1
> +}
> +
> +save_vars { GDBFLAGS } {
> +    clean_restart $testfile
> +}
> +
> +if { [skip_python_tests] } { continue }
> +
> +set pyfile [gdb_remote_download host
> ${srcdir}/${subdir}/py-thread-exited.py]
> +gdb_test_no_output "source ${pyfile}" "load python file"
> +
> +gdb_test "test-events" "Event testers registered."
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +gdb_breakpoint 37 "last of main"
> +
> +gdb_continue_to_breakpoint "continue to breakpoint"
> +
> +gdb_test "python print(threadOneExit)" \
> +        ".*event type: thread-exited. global num: 2.*"
> +gdb_test "python print(threadTwoExit)" \
> +        ".*event type: thread-exited. global num: 3.*"
> diff --git a/gdb/testsuite/gdb.python/py-thread-exited.py
> b/gdb/testsuite/gdb.python/py-thread-exited.py
> new file mode 100644
> index 00000000000..217fcd0ff6b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-thread-exited.py
> @@ -0,0 +1,31 @@
> +import gdb
> +
> +threadOneExit = ""
> +threadTwoExit = ""
> +# we don't want to overwrite the 2nd thread's exit event, thus
> +# store it here. we don't care about it though.
> +mainThreadExit = ""
> +
> +def thread_exited_handler(event):
> +    global threadOneExit, threadTwoExit, mainThreadExit
> +    print("{}".format(event))
> +    assert isinstance(event, gdb.ThreadExitedEvent)
> +    if threadOneExit == "":
> +        threadOneExit = "event type: thread-exited. global num:
> {}".format(event.global_num)
> +    else:
> +        if threadTwoExit == "":
> +            threadTwoExit = "event type: thread-exited. global num:
> {}".format(event.global_num)
> +        else:
> +            mainThreadExit = "event type: thread-exited. global num:
> {}".format(event.global_num)
> +
> +class test_events(gdb.Command):
> +    """Test events."""
> +
> +    def __init__(self):
> +        gdb.Command.__init__(self, "test-events", gdb.COMMAND_STACK)
> +
> +    def invoke(self, arg, from_tty):
> +        gdb.events.thread_exited.connect(thread_exited_handler)
> +        print("Event testers registered.")
> +
> +test_events()
>
> base-commit: bd1c798f0aef38493c5292917e47f76e1205f4e3
> --
> 2.32.0
>
>

Re: [PATCH v4] gdb/Python: Added ThreadExitedEvent

[Tom Tromey]

>>>>> Simon Farre via Gdb-patches <gdb-patches@sourceware.org> writes:

> Tom requested that I should probably just emit the thread object;
> I ran into two issues for this, which I could not resolve in this patch;

> 1 - The Thread Object (the python type) checks it's own validity
> by doing a comparison of it's `thread_info* thread` to nullptr. This
> means that any access of it's attributes may (probably, since we are
> in "async" land) throw Python exceptions because the thread has been
> removed from the thread object.

This seems fine to me, because the event emission comes before the
clearing of the field:

+  if (!evregpy_no_listeners_p (gdb_py_events.thread_exited))
+    {
+      if (emit_thread_exit_event (tmp->thread_obj->thread) < 0)
+	gdbpy_print_stack ();
+    }
   tmp->thread_obj->thread = NULL;

The InferiorThread object is valid until that NULL assignment is done.

> 2 - A python user can hold a global reference to an exiting thread. Thus
> in order to have a ThreadExit event that can provide attribute access
> reliably (both as a global reference, but also inside the thread exit
> handler, as we can never guarantee that it's executed _before_ the
> thread_info pointer is removed from the gdbpy thread object),
> the `thread_info *` thread pointer must not be null. However, this
> comes at the cost of gdb.InferiorThread believing it is "valid" - which means,
> that if a user holds takes a global reference to that
> exiting event thread object, they can some time later do `t.switch()` at which
> point GDB will 'explode' so to speak.

I think it will be fine because thread-switching checks validity using
THPY_REQUIRE_VALID.  So once the underlying (gdb) thread is gone, the
InferiorThread will be in the "invalid" state, and all will be fine.

Another way to look at it is that the user can already stash the
InferiorThread somewhere and refer to it later.

The reason I think it's better to pass the InferiorThread to the event
is that (1) it means you can have a map where the key is a thread and
then it is easy to update when a thread exits, and (2) this doesn't lose
anything because the event can always refer to the members of the thread
if that is what is useful.

Tom