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]

[2/2] FYI: fix PR python/12533


I'm checking this in on the trunk.

PR 12533 points out that a long-running Python command that uses
gdb.Value will use a lot of memory.  This happens because Python doesn't
release values from the value chain.

Earlier, I thought that releasing values like this would cause problems.
But, I no longer think that is correct.

This patch changes the Python value code to release values.  I needed a
funny new 'release_value_or_incref' function to make this work properly
-- Python needs to acquire a new reference and release the value from
the chain, but there is no way at present to accomplish this.

Built and regtested on x86-64 F15.

Tom

2011-12-22  Tom Tromey  <tromey@redhat.com>

	PR python/12533:
	* value.h (release_value_or_incref): Declare.
	* value.c (struct value) <released>: New field.
	(free_all_values, release_value, value_release_to_mark): Update
	'released'.
	(release_value_or_incref): New function.
	* python/py-value.c (valpy_new): Use release_value_or_incref.
	(value_to_value_object): Likewise.
	* varobj.c (install_new_value): Move value_incref earlier.

>From 0dcb2e664747176892c850e05aa717a9643358b6 Mon Sep 17 00:00:00 2001
From: Tom Tromey <tromey@redhat.com>
Date: Wed, 23 Nov 2011 14:23:10 -0700
Subject: [PATCH 2/2] pr 12533

---
 gdb/ChangeLog         |   12 ++++++++++++
 gdb/python/py-value.c |    4 ++--
 gdb/value.c           |   36 ++++++++++++++++++++++++++++++------
 gdb/value.h           |    2 ++
 gdb/varobj.c          |    6 ++++--
 5 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 05e592f..04e355a 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -150,7 +150,7 @@ valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *keywords)
     }
 
   value_obj->value = value;
-  value_incref (value);
+  release_value_or_incref (value);
   value_obj->address = NULL;
   value_obj->type = NULL;
   value_obj->dynamic_type = NULL;
@@ -1123,7 +1123,7 @@ value_to_value_object (struct value *val)
   if (val_obj != NULL)
     {
       val_obj->value = val;
-      value_incref (val);
+      release_value_or_incref (val);
       val_obj->address = NULL;
       val_obj->type = NULL;
       val_obj->dynamic_type = NULL;
diff --git a/gdb/value.c b/gdb/value.c
index b0aa415..d02bc27 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -208,6 +208,9 @@ struct value
      used instead of read_memory to enable extra caching.  */
   unsigned int stack : 1;
 
+  /* If the value has been released.  */
+  unsigned int released : 1;
+
   /* Location of value (if lval).  */
   union
   {
@@ -1210,6 +1213,7 @@ value_free_to_mark (struct value *mark)
   for (val = all_values; val && val != mark; val = next)
     {
       next = val->next;
+      val->released = 1;
       value_free (val);
     }
   all_values = val;
@@ -1228,6 +1232,7 @@ free_all_values (void)
   for (val = all_values; val; val = next)
     {
       next = val->next;
+      val->released = 1;
       value_free (val);
     }
 
@@ -1260,6 +1265,7 @@ release_value (struct value *val)
     {
       all_values = val->next;
       val->next = NULL;
+      val->released = 1;
       return;
     }
 
@@ -1269,11 +1275,26 @@ release_value (struct value *val)
 	{
 	  v->next = val->next;
 	  val->next = NULL;
+	  val->released = 1;
 	  break;
 	}
     }
 }
 
+/* If the value is not already released, release it.
+   If the value is already released, increment its reference count.
+   That is, this function ensures that the value is released from the
+   value chain and that the caller owns a reference to it.  */
+
+void
+release_value_or_incref (struct value *val)
+{
+  if (val->released)
+    value_incref (val);
+  else
+    release_value (val);
+}
+
 /* Release all values up to mark  */
 struct value *
 value_release_to_mark (struct value *mark)
@@ -1282,12 +1303,15 @@ value_release_to_mark (struct value *mark)
   struct value *next;
 
   for (val = next = all_values; next; next = next->next)
-    if (next->next == mark)
-      {
-	all_values = next->next;
-	next->next = NULL;
-	return val;
-      }
+    {
+      if (next->next == mark)
+	{
+	  all_values = next->next;
+	  next->next = NULL;
+	  return val;
+	}
+      next->released = 1;
+    }
   all_values = 0;
   return val;
 }
diff --git a/gdb/value.h b/gdb/value.h
index d2c58ec..167847f 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -772,6 +772,8 @@ extern void free_value_chain (struct value *v);
 
 extern void release_value (struct value *val);
 
+extern void release_value_or_incref (struct value *val);
+
 extern int record_latest_value (struct value *val);
 
 extern void modify_field (struct type *type, gdb_byte *addr,
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 7c68a93..0d5987c 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1604,6 +1604,10 @@ install_new_value (struct varobj *var, struct value *value, int initial)
 	}
     }
 
+  /* Get a reference now, before possibly passing it to any Python
+     code that might release it.  */
+  if (value != NULL)
+    value_incref (value);
 
   /* Below, we'll be comparing string rendering of old and new
      values.  Don't get string rendering if the value is
@@ -1671,8 +1675,6 @@ install_new_value (struct varobj *var, struct value *value, int initial)
   if (var->value != NULL && var->value != value)
     value_free (var->value);
   var->value = value;
-  if (value != NULL)
-    value_incref (value);
   if (value && value_lazy (value) && intentionally_not_fetched)
     var->not_fetched = 1;
   else
-- 
1.7.6.4


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