diff mbox series

Hold the GIL while clearing gdbpy_reggroup_object_map

Message ID 20260324150407.3090217-1-tromey@adacore.com
State New
Headers
Series Hold the GIL while clearing gdbpy_reggroup_object_map |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom Tromey March 24, 2026, 3:04 p.m. UTC 
  Tom de Vries pointed out that a certain Python test left a core file.
Looking into this, I think the problem is that the GIL is not held
when gdbpy_reggroup_object_map is destroyed.

This patch changes the code to explicitly clear the global while
holding the GIL.  This fixed the crash for me.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34013
---
 gdb/python/py-registers.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)


base-commit: 87d2019c27d8af59ce602ba162e7b11852d60f3b

Comments

Tom de Vries March 24, 2026, 4:02 p.m. UTC | #1 
On 3/24/26 4:04 PM, Tom Tromey wrote:
> +static void
> +gdbpy_finalize_registers ()
> +{
> +  /* Have to hold the GIL while clearing the global map.  */
> +  gdbpy_enter enter_py;
> +  gdbpy_reggroup_object_map.clear ();
> +}
> +
> +GDBPY_INITIALIZE_FILE (gdbpy_initialize_registers, gdbpy_finalize_registers);

Using gdbpy_enter doesn't seem to be causing problems here, but we're 
calling this function from finalize_python, at a point where we've 
already called gdbpy_enter::finalize (), so maybe that should trigger an 
error?

Thanks,
- Tom
Tom Tromey March 24, 2026, 5:14 p.m. UTC | #2 
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Using gdbpy_enter doesn't seem to be causing problems here, but we're
Tom> calling this function from finalize_python, at a point where we've
Tom> already called gdbpy_enter::finalize (), so maybe that should trigger
Tom> an error?

It turns out gdbpy_enter isn't needed.
I'll send a v2 that removes this and explains why.

Tom
diff mbox series

Patch

diff --git a/gdb/python/py-registers.c b/gdb/python/py-registers.c
index 44bc53e6a9d..f4e557c7de6 100644
--- a/gdb/python/py-registers.c
+++ b/gdb/python/py-registers.c
@@ -80,6 +80,13 @@  struct reggroup_object : public PyObject
 
 extern PyTypeObject reggroup_object_type;
 
+/* Map from GDB's internal reggroup objects to the Python
+   representation.  GDB's reggroups are global, and are never deleted,
+   so using a map like this is safe.  Note it is explicitly cleaned at
+   Python shutdown, see gdbpy_finalize_registers.  */
+static gdb::unordered_map<const struct reggroup *,gdbpy_ref<>>
+    gdbpy_reggroup_object_map;
+
 /* Return a gdb.RegisterGroup object wrapping REGGROUP.  The register
    group objects are cached, and the same Python object will always be
    returned for the same REGGROUP pointer.  */
@@ -87,12 +94,6 @@  extern PyTypeObject reggroup_object_type;
 static gdbpy_ref<>
 gdbpy_get_reggroup (const reggroup *reggroup)
 {
-  /* Map from GDB's internal reggroup objects to the Python representation.
-     GDB's reggroups are global, and are never deleted, so using a map like
-     this is safe.  */
-  static gdb::unordered_map<const struct reggroup *,gdbpy_ref<>>
-    gdbpy_reggroup_object_map;
-
   /* If there is not already a suitable Python object in the map then
      create a new one, and add it to the map.  */
   if (gdbpy_reggroup_object_map[reggroup] == nullptr)
@@ -433,7 +434,15 @@  gdbpy_initialize_registers ()
   return 0;
 }
 
-GDBPY_INITIALIZE_FILE (gdbpy_initialize_registers);
+static void
+gdbpy_finalize_registers ()
+{
+  /* Have to hold the GIL while clearing the global map.  */
+  gdbpy_enter enter_py;
+  gdbpy_reggroup_object_map.clear ();
+}
+
+GDBPY_INITIALIZE_FILE (gdbpy_initialize_registers, gdbpy_finalize_registers);