7 files changed, 105 insertions, 208 deletions

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 60fec0a..0571469 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -128,6 +128,20 @@ remove_mi_cmd_entry (const std::string &name)
   return true;
 }
 
+/* See mi-cmds.h.  */
+
+void
+remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback)
+{
+  for (auto it = mi_cmd_table.cbegin (); it != mi_cmd_table.cend (); )
+    {
+      if (callback (it->second.get ()))
+	it = mi_cmd_table.erase (it);
+      else
+	++it;
+    }
+}
+
 /* Create and register a new MI command with an MI specific implementation.
    NAME must name an MI command that does not already exist, otherwise an
    assertion will trigger.  */
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 05b702f..9ffb11b 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -22,6 +22,7 @@
 #ifndef MI_MI_CMDS_H
 #define MI_MI_CMDS_H
 
+#include "gdbsupport/function-view.h"
 #include "gdbsupport/gdb_optional.h"
 #include "mi/mi-main.h"
 
@@ -218,5 +219,11 @@ extern bool insert_mi_cmd_entry (mi_command_up command);
 
 extern bool remove_mi_cmd_entry (const std::string &name);
 
+/* Call CALLBACK for each registered MI command.  Remove commands for which
+   CALLBACK returns true.  */
+
+using remove_mi_cmd_entries_ftype
+  = gdb::function_view<bool (mi_command *)>;
+extern void remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback);
 
 #endif /* MI_MI_CMDS_H */
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 3294583..5f63bce 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -82,10 +82,6 @@ frame_filters = {}
 # Initial frame unwinders.
 frame_unwinders = []
 
-# Dictionary containing all user created MI commands, the key is the
-# command name, and the value is the gdb.MICommand object.
-_mi_commands = {}
-
 
 def _execute_unwinders(pending_frame):
     """Internal function called from GDB to execute all unwinders.
diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
index 399e89b..a194798 100644
--- a/gdb/python/py-micmd.c
+++ b/gdb/python/py-micmd.c
@@ -88,9 +88,10 @@ struct mi_command_py : public mi_command
 
   mi_command_py (const char *name, micmdpy_object *object)
     : mi_command (name, nullptr),
-      m_pyobj (object)
+      m_pyobj (gdbpy_ref<micmdpy_object>::new_reference (object))
   {
     pymicmd_debug_printf ("this = %p", this);
+    m_pyobj->mi_command = this;
   }
 
   ~mi_command_py ()
@@ -117,21 +118,42 @@ struct mi_command_py : public mi_command
      then always returns true.  */
   static void validate_installation (micmdpy_object *cmd_obj);
 
-  /* Update m_pyobj to NEW_PYOBJ.  The pointer from M_PYOBJ that points
+  /* Update M_PYOBJ to NEW_PYOBJ.  The pointer from M_PYOBJ that points
      back to this object is swapped with the pointer in NEW_PYOBJ, which
      must be nullptr, so that NEW_PYOBJ now points back to this object.
-     Additionally our parent's name string is stored in m_pyobj, so we
+     Additionally our parent's name string is stored in M_PYOBJ, so we
      swap the name string with NEW_PYOBJ.
 
-     Before this call m_pyobj is the Python object representing this MI
+     Before this call M_PYOBJ is the Python object representing this MI
      command object.  After this call has completed, NEW_PYOBJ now
      represents this MI command object.  */
   void swap_python_object (micmdpy_object *new_pyobj)
   {
+    /* Current object has a backlink, new object doesn't have a backlink.  */
+    gdb_assert (m_pyobj->mi_command != nullptr);
     gdb_assert (new_pyobj->mi_command == nullptr);
+
+    /* Clear the current M_PYOBJ's backlink, set NEW_PYOBJ's backlink.  */
     std::swap (new_pyobj->mi_command, m_pyobj->mi_command);
+
+    /* Both object have names.  */
+    gdb_assert (m_pyobj->mi_command_name != nullptr);
+    gdb_assert (new_pyobj->mi_command_name != nullptr);
+
+    /* mi_command::m_name is the string owned by the current object.  */
+    gdb_assert (m_pyobj->mi_command_name == this->name ());
+
+    /* The name in mi_command::m_name is owned by the current object.  Rather
+       than changing the value of mi_command::m_name (which is not accessible
+       from here) to point to the name owned by the new object, swap the names
+       of the two objects, since we know they are identical strings.  */
+    gdb_assert (strcmp (new_pyobj->mi_command_name,
+			m_pyobj->mi_command_name) == 0);
     std::swap (new_pyobj->mi_command_name, m_pyobj->mi_command_name);
-    m_pyobj = new_pyobj;
+
+    /* Take a reference to the new object, drop the reference to the current
+       object.  */
+    m_pyobj = gdbpy_ref<micmdpy_object>::new_reference (new_pyobj);
   }
 
   /* Called when the MI command is invoked.  */
@@ -139,9 +161,11 @@ struct mi_command_py : public mi_command
 
 private:
   /* The Python object representing this MI command.  */
-  micmdpy_object *m_pyobj;
+  gdbpy_ref<micmdpy_object> m_pyobj;
 };
 
+using mi_command_py_up = std::unique_ptr<mi_command_py>;
+
 extern PyTypeObject micmdpy_object_type
 	CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("micmdpy_object");
 
@@ -321,8 +345,6 @@ mi_command_py::invoke (struct mi_parse *parse) const
   if (parse->argv == nullptr)
     error (_("Problem parsing arguments: %s %s"), parse->command, parse->args);
 
-  PyObject *obj = (PyObject *) this->m_pyobj;
-  gdb_assert (obj != nullptr);
 
   gdbpy_enter enter_py;
 
@@ -341,9 +363,11 @@ mi_command_py::invoke (struct mi_parse *parse) const
 	gdbpy_handle_exception ();
     }
 
+  gdb_assert (this->m_pyobj != nullptr);
   gdb_assert (PyErr_Occurred () == nullptr);
-  gdbpy_ref<> result (PyObject_CallMethodObjArgs (obj, invoke_cst,
-						  argobj.get (), nullptr));
+  gdbpy_ref<> result
+    (PyObject_CallMethodObjArgs ((PyObject *) this->m_pyobj.get (), invoke_cst,
+				 argobj.get (), nullptr));
   if (result == nullptr)
     gdbpy_handle_exception ();
 
@@ -367,32 +391,13 @@ mi_command_py::validate_installation (micmdpy_object *cmd_obj)
   gdb_assert (cmd->m_pyobj == cmd_obj);
 }
 
-/* Return a reference to the gdb._mi_commands dictionary.  If the
-   dictionary can't be found for any reason then nullptr is returned, and
-   a Python exception will be set.  */
+/* Return CMD as an mi_command_py if it is a Python MI command, else
+   nullptr.  */
 
-static gdbpy_ref<>
-micmdpy_global_command_dictionary ()
+static mi_command_py *
+as_mi_command_py (mi_command *cmd)
 {
-  if (gdb_python_module == nullptr)
-    {
-      PyErr_SetString (PyExc_RuntimeError, _("unable to find gdb module"));
-      return nullptr;
-    }
-
-  gdbpy_ref<> mi_cmd_dict (PyObject_GetAttrString (gdb_python_module,
-						   "_mi_commands"));
-  if (mi_cmd_dict == nullptr)
-    return nullptr;
-
-  if (!PyDict_Check (mi_cmd_dict.get ()))
-    {
-      PyErr_SetString (PyExc_RuntimeError,
-		       _("gdb._mi_commands is not a dictionary as expected"));
-      return nullptr;
-    }
-
-  return mi_cmd_dict;
+  return dynamic_cast<mi_command_py *> (cmd);
 }
 
 /* Uninstall OBJ, making the MI command represented by OBJ unavailable for
@@ -409,41 +414,13 @@ micmdpy_uninstall_command (micmdpy_object *obj)
 
   pymicmd_debug_printf ("name = %s", obj->mi_command_name);
 
-  /* Remove the command from the internal MI table of commands, this will
-     cause the c++ object to be deleted, which will clear the mi_command
-     member variable within the Python object.  */
-  remove_mi_cmd_entry (obj->mi_command->name ());
+  /* Remove the command from the internal MI table of commands.  This will
+     cause the mi_command_py object to be deleted, which will clear the
+     backlink in OBJ.  */
+  bool removed = remove_mi_cmd_entry (obj->mi_command->name ());
+  gdb_assert (removed);
   gdb_assert (obj->mi_command == nullptr);
 
-  gdbpy_ref<> mi_cmd_dict = micmdpy_global_command_dictionary ();
-  if (mi_cmd_dict == nullptr)
-    return -1;
-
-  /* Grab the name for this command.  */
-  gdbpy_ref<> name_obj
-    = host_string_to_python_string (obj->mi_command_name);
-  if (name_obj == nullptr)
-    return -1;
-
-  /* Lookup the gdb.MICommand object in the dictionary of all Python MI
-     commands, this is gdb._mi_command, and remove it.  */
-  PyObject *curr = PyDict_GetItemWithError (mi_cmd_dict.get (),
-					    name_obj.get ());
-
-  /* Did we encounter an error?  Failing to find the object in the
-     dictionary isn't an error, that's fine.  */
-  if (curr == nullptr && PyErr_Occurred ())
-    return -1;
-
-  /* Did we find this command in the gdb._mi_commands dictionary?  If so,
-     then remove it.  */
-  if (curr != nullptr)
-    {
-      /* Yes we did!  Remove it.  */
-      if (PyDict_DelItem (mi_cmd_dict.get (), name_obj.get ()) < 0)
-	return -1;
-    }
-
   return 0;
 }
 
@@ -467,96 +444,36 @@ micmdpy_install_command (micmdpy_object *obj)
 
   pymicmd_debug_printf ("name = %s", obj->mi_command_name);
 
-  gdbpy_ref<> mi_cmd_dict = micmdpy_global_command_dictionary ();
-  if (mi_cmd_dict == nullptr)
-    return -1;
-
-  /* Look up this command name in the gdb._mi_commands dictionary, a
-     command with this name may already exist.  */
-  gdbpy_ref<> name_obj
-    = host_string_to_python_string (obj->mi_command_name);
+  /* Look up this command name in MI_COMMANDS, a command with this name may
+     already exist.  */
+  mi_command *cmd = mi_cmd_lookup (obj->mi_command_name);
+  mi_command_py *cmd_py = as_mi_command_py (cmd);
 
-  PyObject *curr = PyDict_GetItemWithError (mi_cmd_dict.get (),
-					    name_obj.get ());
-  if (curr == nullptr && PyErr_Occurred ())
-    return -1;
-  if (curr != nullptr)
+  if (cmd != nullptr && cmd_py == nullptr)
     {
-      /* There is a command with this name already in the gdb._mi_commands
-	 dictionary.  First, validate that the object in the dictionary is
-	 of the expected type, just in case something weird has happened.  */
-      if (!PyObject_IsInstance (curr, (PyObject *) &micmdpy_object_type))
-	{
-	  PyErr_SetString (PyExc_RuntimeError,
-			   _("unexpected object in gdb._mi_commands dictionary"));
-	  return -1;
-	}
-
-      /* To get to this function OBJ should not be installed, which should
-	 mean OBJ is not in the gdb._mi_commands dictionary.  If we find
-	 that OBJ is the thing in the dictionary, then something weird is
-	 going on, we should throw an error.  */
-      micmdpy_object *other = (micmdpy_object *) curr;
-      if (other == obj || other->mi_command == nullptr)
-	{
-	  PyErr_SetString (PyExc_RuntimeError,
-			   _("uninstalled command found in gdb._mi_commands dictionary"));
-	  return -1;
-	}
-
-      /* All Python mi command object should always have a name set.  */
-      gdb_assert (other->mi_command_name != nullptr);
-
-      /* We always insert commands into the gdb._mi_commands dictionary
-	 using their name as a key, if this check fails then the dictionary
-	 is in some weird state.  */
-      if (other->mi_command_name != other->mi_command->name ()
-	  || strcmp (other->mi_command_name, obj->mi_command_name) != 0)
-	{
-	  PyErr_SetString (PyExc_RuntimeError,
-			   _("gdb._mi_commands dictionary is corrupted"));
-	  return -1;
-	}
+      /* There is already an MI command registered with that name, and it's not
+         a Python one.  Forbid replacing a non-Python MI command.  */
+      PyErr_SetString (PyExc_RuntimeError,
+		       _("unable to add command, name is already in use"));
+      return -1;
+    }
 
-      /* Switch the state of the c++ object held in the MI command table
-	 so that it now references OBJ.  After this action the old Python
-	 object that used to be referenced from the MI command table will
-	 now show as uninstalled, while the new Python object will show as
-	 installed.  */
-      other->mi_command->swap_python_object (obj);
-
-      gdb_assert (other->mi_command == nullptr);
-      gdb_assert (obj->mi_command != nullptr);
-      gdb_assert (obj->mi_command->name () == obj->mi_command_name);
-
-      /* Remove the previous Python object from the gdb._mi_commands
-	 dictionary, we'll install the new object below.  */
-      if (PyDict_DelItem (mi_cmd_dict.get (), name_obj.get ()) < 0)
-       return -1;
+  if (cmd_py != nullptr)
+    {
+      /* There is already a Python MI command registered with that name, swap
+         in the new gdb.MICommand implementation.  */
+      cmd_py->swap_python_object (obj);
     }
   else
     {
-      /* There's no Python object for this command name in the
-	 gdb._mi_commands dictionary from which we can steal an existing
-	 object already held in the MI commands table, and so, we now
-	 create a new c++ object, and install it into the MI table.  */
-      obj->mi_command = new mi_command_py (obj->mi_command_name, obj);
-      mi_command_up micommand (obj->mi_command);
+      /* There's no MI command registered with that name at all, create one.  */
+      mi_command_py_up mi_cmd (new mi_command_py (obj->mi_command_name, obj));
 
       /* Add the command to the gdb internal MI command table.  */
-      bool result = insert_mi_cmd_entry (std::move (micommand));
-      if (!result)
-	{
-	  PyErr_SetString (PyExc_RuntimeError,
-			   _("unable to add command, name may already be in use"));
-	  return -1;
-	}
+      bool result = insert_mi_cmd_entry (std::move (mi_cmd));
+      gdb_assert (result);
     }
 
-  /* Finally, add the Python object to the gdb._mi_commands dictionary.  */
-  if (PyDict_SetItem (mi_cmd_dict.get (), name_obj.get (), (PyObject *) obj) < 0)
-    return -1;
-
   return 0;
 }
 
@@ -662,11 +579,10 @@ micmdpy_dealloc (PyObject *obj)
 			(cmd->mi_command_name == nullptr
 			 ? "(null)" : cmd->mi_command_name));
 
-  /* Remove the command from the MI command table if needed.  This will
-     cause the mi_command_py object to be deleted, which, in turn, will
-     clear the cmd->mi_command member variable, hence the assert.  */
-  if (cmd->mi_command != nullptr)
-    remove_mi_cmd_entry (cmd->mi_command->name ());
+  /* As the mi_command_py object holds a reference to the micmdpy_object,
+     the only way the dealloc function can be called is if the mi_command_py
+     object has been deleted, in which case the following assert will
+     hold.  */
   gdb_assert (cmd->mi_command == nullptr);
 
   /* Free the memory that holds the command name.  */
@@ -698,6 +614,18 @@ gdbpy_initialize_micommands ()
   return 0;
 }
 
+void
+gdbpy_finalize_micommands ()
+{
+  /* mi_command_py objects hold references to micmdpy_object objects.  They must
+     be dropped before the Python interpreter is finalized.  Do so by removing
+     those MI command entries, thus deleting the mi_command_py objects.  */
+  remove_mi_cmd_entries ([] (mi_command *cmd)
+    {
+      return as_mi_command_py (cmd) != nullptr;
+    });
+}
+
 /* Get the gdb.MICommand.name attribute, returns a string, the name of this
    MI command.  */
 
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 083c4db..098d913 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -564,6 +564,7 @@ int gdbpy_initialize_connection ()
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 int gdbpy_initialize_micommands (void)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+void gdbpy_finalize_micommands ();
 
 /* A wrapper for PyErr_Fetch that handles reference counting for the
    caller.  */
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 9795588..91636ef 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1795,6 +1795,8 @@ finalize_python (void *ignore)
   (void) PyGILState_Ensure ();
   gdbpy_enter::finalize ();
 
+  gdbpy_finalize_micommands ();
+
   Py_Finalize ();
 
   gdb_python_initialized = false;
diff --git a/gdb/testsuite/gdb.python/py-mi-cmd.exp b/gdb/testsuite/gdb.python/py-mi-cmd.exp
index c102efb..300ab95 100644
--- a/gdb/testsuite/gdb.python/py-mi-cmd.exp
+++ b/gdb/testsuite/gdb.python/py-mi-cmd.exp
@@ -289,31 +289,6 @@ mi_gdb_test "-aa" \
     ".*\\^done,zzz={msg=\"message three\"}" \
     "call the -aa command looking for message three"
 
-# Remove the gdb._mi_commands dictionary, then try to register a new
-# command.
-mi_gdb_test "python del(gdb._mi_commands)" ".*\\^done"
-mi_gdb_test "python pycmd3('-hello', 'Hello', 'msg')" \
-    [multi_line \
-	 ".*" \
-	 "&\"AttributeError: module 'gdb' has no attribute '_mi_commands'..\"" \
-	 "&\"Error while executing Python code\\...\"" \
-	 "\\^error,msg=\"Error while executing Python code\\.\""] \
-    "register a command with no gdb._mi_commands available"
-
-# Set gdb._mi_commands to be something other than a dictionary, and
-# try to register a command.
-mi_gdb_test "python gdb._mi_commands = 'string'" ".*\\^done"
-mi_gdb_test "python pycmd3('-hello', 'Hello', 'msg')" \
-    [multi_line \
-	 ".*" \
-	 "&\"RuntimeError: gdb._mi_commands is not a dictionary as expected..\"" \
-	 "&\"Error while executing Python code\\...\"" \
-	 "\\^error,msg=\"Error while executing Python code\\.\""] \
-    "register a command when gdb._mi_commands is not a dictionary"
-
-# Restore gdb._mi_commands to a dictionary.
-mi_gdb_test "python gdb._mi_commands = {}" ".*\\^done"
-
 # Try to register a command object that is missing an invoke method.
 # This is accepted, but will give an error when the user tries to run
 # the command.
@@ -346,37 +321,11 @@ mi_gdb_test "-hello" \
 	 "\\^error,msg=\"Error occurred in Python: 'str' object is not callable\""] \
     "execute command with invoke set to a string"
 
-# Further checking of corruption to the gdb._mi_commands dictionary.
-#
-# First, insert an object of the wrong type, then try to register an
-# MI command that will go into that same dictionary slot.
-mi_gdb_test "python gdb._mi_commands\['blah'\] = 'blah blah blah'" ".*\\^done"
-mi_gdb_test "python pycmd2('-blah')" \
-    [multi_line \
-	 ".*" \
-	 "&\"RuntimeError: unexpected object in gdb\\._mi_commands dictionary..\"" \
-	 "&\"Error while executing Python code\\...\"" \
-	 "\\^error,msg=\"Error while executing Python code\\.\""] \
-    "hit unexpected object in gdb._mi_commands dictionary"
-
-# Next, create a command, uninstall it, then force the command back
-# into the dictionary.
-mi_gdb_test "python cmd = pycmd2('-foo')" ".*\\^done"
-mi_gdb_test "python cmd.installed = False" ".*\\^done"
-mi_gdb_test "python gdb._mi_commands\['foo'\] = cmd" ".*\\^done"
-mi_gdb_test "python cmd.installed = True" \
-    [multi_line \
-	 ".*" \
-	 "&\"RuntimeError: uninstalled command found in gdb\\._mi_commands dictionary..\"" \
-	 "&\"Error while executing Python code\\...\"" \
-	 "\\^error,msg=\"Error while executing Python code\\.\""] \
-    "found uninstalled command in gdb._mi_commands dictionary"
-
 # Try to create a new MI command that uses the name of a builtin MI command.
 mi_gdb_test "python cmd = pycmd2('-data-disassemble')" \
     [multi_line \
 	 ".*" \
-	 "&\"RuntimeError: unable to add command, name may already be in use..\"" \
+	 "&\"RuntimeError: unable to add command, name is already in use..\"" \
 	 "&\"Error while executing Python code\\...\"" \
 	 "\\^error,msg=\"Error while executing Python code\\.\""] \
     "try to register a command that replaces -data-disassemble"