Reduce manual reference counting in py-inferior.c

Message ID 20190701150747.19075-1-tromey@adacore.com
State New, archived
Headers

Commit Message

Tom Tromey July 1, 2019, 3:07 p.m. UTC
  This patch changes py-inferior.c to use gdbpy_ref<> when possible,
reducing the amount of manual reference counting.

Tested on x86-64 Fedora 29.

gdb/ChangeLog
2019-07-01  Tom Tromey  <tromey@adacore.com>

	* python/python-internal.h (create_thread_object): Return
	gdbpy_ref.
	* python/py-infthread.c (create_thread_object): Return gdbpy_ref.
	* python/py-inferior.c (struct threadlist_entry): Add
	constructor.
	<thread_obj>: Now a gdbpy_ref.
	(thread_to_thread_object): Update.
	(add_thread_object): Use new.
	(delete_thread_object): Use delete.
	(infpy_threads): Update.
	(py_free_inferior): Update.  Construct "inf_obj" after acquiring
	GIL.
---
 gdb/ChangeLog                | 15 +++++++++++++++
 gdb/python/py-inferior.c     | 36 +++++++++++++++++++-----------------
 gdb/python/py-infthread.c    |  8 ++++----
 gdb/python/python-internal.h |  2 +-
 4 files changed, 39 insertions(+), 22 deletions(-)
  

Comments

Tom Tromey July 10, 2019, 6:24 p.m. UTC | #1
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> This patch changes py-inferior.c to use gdbpy_ref<> when possible,
Tom> reducing the amount of manual reference counting.

Tom> Tested on x86-64 Fedora 29.

Tom> gdb/ChangeLog
Tom> 2019-07-01  Tom Tromey  <tromey@adacore.com>

Tom> 	* python/python-internal.h (create_thread_object): Return
Tom> 	gdbpy_ref.
Tom> 	* python/py-infthread.c (create_thread_object): Return gdbpy_ref.
Tom> 	* python/py-inferior.c (struct threadlist_entry): Add
Tom> 	constructor.
Tom> 	<thread_obj>: Now a gdbpy_ref.
Tom> 	(thread_to_thread_object): Update.
Tom> 	(add_thread_object): Use new.
Tom> 	(delete_thread_object): Use delete.
Tom> 	(infpy_threads): Update.
Tom> 	(py_free_inferior): Update.  Construct "inf_obj" after acquiring
Tom> 	GIL.

I'm checking this in.

Tom
  
Tom Tromey July 11, 2019, 2:57 p.m. UTC | #2
Tom> This patch changes py-inferior.c to use gdbpy_ref<> when possible,
Tom> reducing the amount of manual reference counting.

Tom> Tested on x86-64 Fedora 29.

I must have tested incorrectly somehow, because I'm seeing a test
failure now.  I will have a fix ready shortly.

Tom
  

Patch

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 7e7d518c557..9a979897eb0 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -30,8 +30,14 @@ 
 #include "py-event.h"
 #include "py-stopevent.h"
 
-struct threadlist_entry {
-  thread_object *thread_obj;
+struct threadlist_entry
+{
+  threadlist_entry (gdbpy_ref<thread_object> &&ref)
+    : thread_obj (std::move (ref))
+  {
+  }
+
+  gdbpy_ref<thread_object> thread_obj;
   struct threadlist_entry *next;
 };
 
@@ -301,7 +307,7 @@  thread_to_thread_object (thread_info *thr)
        thread != NULL;
        thread = thread->next)
     if (thread->thread_obj->thread == thr)
-      return gdbpy_ref<>::new_reference ((PyObject *) thread->thread_obj);
+      return gdbpy_ref<>::new_reference ((PyObject *) thread->thread_obj.get ());
 
   PyErr_SetString (PyExc_SystemError,
 		   _("could not find gdb thread object"));
@@ -311,7 +317,6 @@  thread_to_thread_object (thread_info *thr)
 static void
 add_thread_object (struct thread_info *tp)
 {
-  thread_object *thread_obj;
   inferior_object *inf_obj;
   struct threadlist_entry *entry;
 
@@ -320,8 +325,8 @@  add_thread_object (struct thread_info *tp)
 
   gdbpy_enter enter_py (python_gdbarch, python_language);
 
-  thread_obj = create_thread_object (tp);
-  if (!thread_obj)
+  gdbpy_ref<thread_object> thread_obj = create_thread_object (tp);
+  if (thread_obj == NULL)
     {
       gdbpy_print_stack ();
       return;
@@ -329,8 +334,7 @@  add_thread_object (struct thread_info *tp)
 
   inf_obj = (inferior_object *) thread_obj->inf_obj;
 
-  entry = XNEW (struct threadlist_entry);
-  entry->thread_obj = thread_obj;
+  entry = new threadlist_entry (std::move (thread_obj));
   entry->next = inf_obj->threads;
 
   inf_obj->threads = entry;
@@ -340,7 +344,7 @@  add_thread_object (struct thread_info *tp)
     return;
 
   gdbpy_ref<> event = create_thread_event_object (&new_thread_event_object_type,
-						  (PyObject *) thread_obj);
+						  (PyObject *) thread_obj.get ());
   if (event == NULL
       || evpy_emit_event (event.get (), gdb_py_events.new_thread) < 0)
     gdbpy_print_stack ();
@@ -375,8 +379,7 @@  delete_thread_object (struct thread_info *tp, int ignore)
   *entry = (*entry)->next;
   inf_obj->nthreads--;
 
-  Py_DECREF (tmp->thread_obj);
-  xfree (tmp);
+  delete tmp;
 }
 
 static PyObject *
@@ -405,8 +408,9 @@  infpy_threads (PyObject *self, PyObject *args)
   for (i = 0, entry = inf_obj->threads; i < inf_obj->nthreads;
        i++, entry = entry->next)
     {
-      Py_INCREF (entry->thread_obj);
-      PyTuple_SET_ITEM (tuple, i, (PyObject *) entry->thread_obj);
+      PyObject *thr = (PyObject *) entry->thread_obj.get ();
+      Py_INCREF (thr);
+      PyTuple_SET_ITEM (tuple, i, thr);
     }
 
   return tuple;
@@ -859,24 +863,22 @@  infpy_dealloc (PyObject *obj)
 static void
 py_free_inferior (struct inferior *inf, void *datum)
 {
-  gdbpy_ref<inferior_object> inf_obj ((inferior_object *) datum);
   struct threadlist_entry *th_entry, *th_tmp;
 
   if (!gdb_python_initialized)
     return;
 
   gdbpy_enter enter_py (python_gdbarch, python_language);
+  gdbpy_ref<inferior_object> inf_obj ((inferior_object *) datum);
 
   inf_obj->inferior = NULL;
 
   /* Deallocate threads list.  */
   for (th_entry = inf_obj->threads; th_entry != NULL;)
     {
-      Py_DECREF (th_entry->thread_obj);
-
       th_tmp = th_entry;
       th_entry = th_entry->next;
-      xfree (th_tmp);
+      delete th_tmp;
     }
 
   inf_obj->nthreads = 0;
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 8c556f92ea8..f1c316c1f17 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -36,17 +36,17 @@  extern PyTypeObject thread_object_type
       }								\
   } while (0)
 
-thread_object *
+gdbpy_ref<thread_object>
 create_thread_object (struct thread_info *tp)
 {
-  thread_object *thread_obj;
+  gdbpy_ref<thread_object> thread_obj;
 
   gdbpy_ref<inferior_object> inf_obj = inferior_to_inferior_object (tp->inf);
   if (inf_obj == NULL)
     return NULL;
 
-  thread_obj = PyObject_New (thread_object, &thread_object_type);
-  if (!thread_obj)
+  thread_obj.reset (PyObject_New (thread_object, &thread_object_type));
+  if (thread_obj == NULL)
     return NULL;
 
   thread_obj->thread = tp;
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 69ff1fe30de..e6a3fe0ec1f 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -468,7 +468,7 @@  PyObject *gdbpy_lookup_objfile (PyObject *self, PyObject *args, PyObject *kw);
 
 PyObject *gdbarch_to_arch_object (struct gdbarch *gdbarch);
 
-thread_object *create_thread_object (struct thread_info *tp);
+gdbpy_ref<thread_object> create_thread_object (struct thread_info *tp);
 gdbpy_ref<> thread_to_thread_object (thread_info *thr);;
 gdbpy_ref<inferior_object> inferior_to_inferior_object (inferior *inf);