This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH 4/5] Make Python inferior-related internal functions return a gdbpy_inf_ref


From: Simon Marchi <simon.marchi@polymtl.ca>

The functions inferior_to_inferior_object and find_inferior_object
return a new reference to an inferior_object.  This means that the
caller owns that reference and is responsible for decrementing it when
it's done.  To avoid the possibility of the caller forgetting to DECREF
when it's done with the reference, make those functions return a
gdbpy_inf_ref instead of a plain pointer.

If the caller doesn't need the reference after it has used it,
gdbpy_inf_ref will take care of removing that reference.  If the
reference needs to outlive the gdbpy_inf_ref object (e.g. because we are
return the value to Python, which will take ownership of the reference),
the caller will have to release the pointer.  At least it will be
explicit and it won't be ambiguous.

I added comments in inferior_to_inferior_object for the poor souls who
will have to deal with this again in the future.

A couple of things I am not sure about:

  * I am not sure whether the behaviour is right with the assignment
  operator in delete_thread_object, so if somebody could take a look at
  that in particular it would be appreciated:

    gdbpy_inf_ref inf_obj_ref = find_inferior_object (ptid_get_pid (tp->ptid));

  I suppose it's the operator= version which moves the reference that is
  invoked?

  * I used .release() on the reference in create_thread_object with a
  comment explaining why, but I would need some more pairs of eyes on
  that to see if it's right.

gdb/ChangeLog:

	* python/py-inferior.c (inferior_to_inferior_object): Return a
	gdbpy_inf_ref.
	(find_inferior_object): Return a gdbpy_inf_ref.
	(delete_thread_object): Use gdbpy_inf_ref.
	(gdbpy_selected_inferior): Likewise.
	* python/py-infthread.c (create_thread_object): Adapt to
	gdbpy_inf_ref.
	* python/python-internal.h: Include py-ref.h.
	(inferior_to_inferior_object): Return a gdbpy_inf_ref.
	(find_inferior_object): Likewise.
---
 gdb/python/py-inferior.c     | 41 +++++++++++++++++++----------------------
 gdb/python/py-infthread.c    |  6 +++++-
 gdb/python/py-ref.h          |  2 ++
 gdb/python/python-internal.h |  6 ++++--
 4 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index b6b43af7cd..340dddcfbd 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -207,39 +207,38 @@ python_new_objfile (struct objfile *objfile)
    representing INFERIOR.  If the object has already been created,
    return it and increment the reference count,  otherwise, create it.
    Return NULL on failure.  */
-inferior_object *
+gdbpy_inf_ref
 inferior_to_inferior_object (struct inferior *inferior)
 {
   inferior_object *inf_obj;
 
   inf_obj = (inferior_object *) inferior_data (inferior, infpy_inf_data_key);
-  if (!inf_obj)
+  if (inf_obj == NULL)
     {
       if (debug_python)
 	printf_filtered ("Creating Python Inferior object inf = %d\n",
 			 inferior->num);
 
       inf_obj = PyObject_New (inferior_object, &inferior_object_type);
-      if (!inf_obj)
-	  return NULL;
+      if (inf_obj == NULL)
+	return gdbpy_inf_ref ();
 
       inf_obj->inferior = inferior;
       inf_obj->threads = NULL;
       inf_obj->nthreads = 0;
 
       set_inferior_data (inferior, infpy_inf_data_key, inf_obj);
-
     }
   else
     Py_INCREF ((PyObject *)inf_obj);
 
-  return inf_obj;
+  return gdbpy_inf_ref (inf_obj);
 }
 
 /* Finds the Python Inferior object for the given PID.  Returns a
    reference, or NULL if PID does not match any inferior object. */
 
-inferior_object *
+gdbpy_inf_ref
 find_inferior_object (int pid)
 {
   struct inferior *inf = find_inferior_pid (pid);
@@ -247,7 +246,7 @@ find_inferior_object (int pid)
   if (inf)
     return inferior_to_inferior_object (inf);
 
-  return NULL;
+  return gdbpy_inf_ref ();
 }
 
 thread_object *
@@ -304,39 +303,34 @@ add_thread_object (struct thread_info *tp)
 static void
 delete_thread_object (struct thread_info *tp, int ignore)
 {
-  inferior_object *inf_obj;
   struct threadlist_entry **entry, *tmp;
 
   if (!gdb_python_initialized)
     return;
 
   gdbpy_enter enter_py (python_gdbarch, python_language);
+  gdbpy_inf_ref inf_obj_ref = find_inferior_object (ptid_get_pid (tp->ptid));
 
-  inf_obj
-    = (inferior_object *) find_inferior_object (ptid_get_pid (tp->ptid));
-  if (!inf_obj)
+  if (inf_obj_ref == NULL)
     return;
 
   /* Find thread entry in its inferior's thread_list.  */
-  for (entry = &inf_obj->threads; *entry != NULL; entry =
-	 &(*entry)->next)
+  for (entry = &inf_obj_ref.get ()->threads;
+       *entry != NULL;
+       entry = &(*entry)->next)
     if ((*entry)->thread_obj->thread == tp)
       break;
 
-  if (!*entry)
-    {
-      Py_DECREF (inf_obj);
-      return;
-    }
+  if (*entry == NULL)
+    return;
 
   tmp = *entry;
   tmp->thread_obj->thread = NULL;
 
   *entry = (*entry)->next;
-  inf_obj->nthreads--;
+  inf_obj_ref.get ()->nthreads--;
 
   Py_DECREF (tmp->thread_obj);
-  Py_DECREF (inf_obj);
   xfree (tmp);
 }
 
@@ -815,7 +809,10 @@ py_free_inferior (struct inferior *inf, void *datum)
 PyObject *
 gdbpy_selected_inferior (PyObject *self, PyObject *args)
 {
-  return (PyObject *) inferior_to_inferior_object (current_inferior ());
+  gdbpy_inf_ref inf_obj_ref (inferior_to_inferior_object (current_inferior ()));
+
+  /* Release the reference, it will now be managed by Python.  */
+  return (PyObject *) inf_obj_ref.release ();
 }
 
 int
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index c7553310c3..79fb5d12d1 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -46,7 +46,11 @@ create_thread_object (struct thread_info *tp)
     return NULL;
 
   thread_obj->thread = tp;
-  thread_obj->inf_obj = find_inferior_object (ptid_get_pid (tp->ptid));
+
+  /* The thread holds a weak reference to its inferior to avoid creating a
+     reference loop between the inferior and its threads.  */
+  gdbpy_inf_ref inf_obj_ref = find_inferior_object (ptid_get_pid (tp->ptid));
+  thread_obj->inf_obj = inf_obj_ref.release ();
 
   return thread_obj;
 }
diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
index b212ef195f..03c0304c74 100644
--- a/gdb/python/py-ref.h
+++ b/gdb/python/py-ref.h
@@ -67,6 +67,8 @@ public:
 /* Specializations of gdbpy_ref_base for concrete Python object types.  */
 
 typedef gdbpy_ref_base<PyObject> gdbpy_ref;
+
+struct inferior_object;
 typedef gdbpy_ref_base<inferior_object> gdbpy_inf_ref;
 
 #endif /* GDB_PYTHON_REF_H */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 62a834d403..a7fa9aa4bc 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -94,6 +94,8 @@
 #include <Python.h>
 #include <frameobject.h>
 
+#include "py-ref.h"
+
 #if PY_MAJOR_VERSION >= 3
 #define IS_PY3K 1
 #endif
@@ -421,8 +423,8 @@ PyObject *gdbarch_to_arch_object (struct gdbarch *gdbarch);
 thread_object *create_thread_object (struct thread_info *tp);
 thread_object *find_thread_object (ptid_t ptid)
     CPYCHECKER_RETURNS_BORROWED_REF;
-inferior_object *find_inferior_object (int pid);
-inferior_object *inferior_to_inferior_object (struct inferior *inferior);
+gdbpy_inf_ref find_inferior_object (int pid);
+gdbpy_inf_ref inferior_to_inferior_object (struct inferior *inferior);
 
 const struct block *block_object_to_block (PyObject *obj);
 struct symbol *symbol_object_to_symbol (PyObject *obj);
-- 
2.11.0


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]