This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[2/2] FYI: fix PR python/12533
- From: Tom Tromey <tromey at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Thu, 22 Dec 2011 12:49:53 -0700
- Subject: [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