Hold the GIL while clearing gdbpy_reggroup_object_map

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.

This also adds a comment to gdbpy_initialize_file to explicitly
mention that the GIL is held while the finalizers are run.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34013
Reviewed-By: Tom de Vries <tdevries@suse.de>

diff --git a/gdb/python/py-registers.c b/gdb/python/py-registers.c
index 44bc53e6a9d6ef2cabf401995b3247d27a4f5548..c6e0748e935d04b50437a6c826c52c7678d6fcef 100644 (file)
--- 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; this is
+     guaranteed for finalizers.  */
+  gdbpy_reggroup_object_map.clear ();
+}
+
+GDBPY_INITIALIZE_FILE (gdbpy_initialize_registers, gdbpy_finalize_registers);
 
 \f
 

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index bdc960ed20864545d8e9990245766473a08fa74f..b24b9314994b0cbfd027348825a5350c125a334f 100644 (file)
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -616,7 +616,9 @@ class gdbpy_initialize_file
 
      There is no error return in this case.  This function is only called
      when GDB is already shutting down.  The function should make a best
-     effort to clean up, and then return.  */
+     effort to clean up, and then return.
+
+     The GIL will be held while calling the finalizers.  */
 
   using gdbpy_finalize_file_ftype = void (*) (void);