[patch] [5/5] Types reference counting [value_history_cleanup]

[patch] [5/5] Types reference counting [value_history_cleanup]

[Jan Kratochvil]

Hi,

this patch brings no new functionality.  It could be enabled only during some
--enable-debug builds but there is currently no such configure option.

Found it caught some bugs during the development (by trying to double-free some
types).  It can be also used to verify there are no types leaks.


Thanks,
Jan


gdb/
2009-04-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* value.c (value_history_cleanup): New function.
	(_initialize_values): Setup a value_history_cleanup final call.

diff --git a/gdb/value.c b/gdb/value.c
index 9c08a41..12a5662 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -852,6 +852,25 @@ show_values (char *num_exp, int from_tty)
       num_exp[1] = '\0';
     }
 }
+
+/* Sanity check for memory leaks and proper types reference counting.  */
+
+static void
+value_history_cleanup (void *unused)
+{
+  while (value_history_chain)
+    {
+      struct value_history_chunk *chunk = value_history_chain;
+      int i;
+
+      for (i = 0; i < ARRAY_SIZE (chunk->values); i++)
+      	value_free (chunk->values[i]);
+
+      value_history_chain = chunk->next;
+      xfree (chunk);
+    }
+  value_history_count = 0;
+}
 \f
 /* Internal variables.  These are variables within the debugger
    that hold values assigned by debugger commands.
@@ -2049,4 +2068,6 @@ Placeholder command for showing help on convenience functions."),
   TYPE_CODE (internal_fn_type) = TYPE_CODE_INTERNAL_FUNCTION;
   TYPE_LENGTH (internal_fn_type) = sizeof (struct internal_function *);
   TYPE_NAME (internal_fn_type) = "<internal function>";
+
+  make_final_cleanup (value_history_cleanup, NULL);
 }

Re: [patch] [5/5] Types reference counting [value_history_cleanup]

[Jan Kratochvil]

On Sat, 11 Apr 2009 12:22:37 +0200, Jan Kratochvil wrote:
> this patch brings no new functionality.  It could be enabled only during some
> --enable-debug builds but there is currently no such configure option.
> 
> Found it caught some bugs during the development (by trying to double-free some
> types).  It can be also used to verify there are no types leaks.

There is now a fix to free anything at all.  When the function was written
there was an older implementation of type_decref which was incorrectly freeing
the zero-reference-count types immediately (and not just on the garbage
collection by free_all_types).

gdb/
2009-04-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* value.c (value_history_cleanup): New function.
	(_initialize_values): Setup a value_history_cleanup final call.

diff --git a/gdb/value.c b/gdb/value.c
index 76ba93b..c5df1d3 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -861,6 +861,29 @@ show_values (char *num_exp, int from_tty)
       num_exp[1] = '\0';
     }
 }
+
+/* Sanity check for memory leaks and proper types reference counting.  */
+
+static void
+value_history_cleanup (void *unused)
+{
+  while (value_history_chain)
+    {
+      struct value_history_chunk *chunk = value_history_chain;
+      int i;
+
+      for (i = 0; i < ARRAY_SIZE (chunk->values); i++)
+      	value_free (chunk->values[i]);
+
+      value_history_chain = chunk->next;
+      xfree (chunk);
+    }
+  value_history_count = 0;
+
+  /* Free the unreferenced types above.  */
+  free_all_values ();
+  free_all_types ();
+}
 \f
 /* Internal variables.  These are variables within the debugger
    that hold values assigned by debugger commands.
@@ -2071,4 +2094,6 @@ Placeholder command for showing help on convenience functions."),
   TYPE_CODE (internal_fn_type) = TYPE_CODE_INTERNAL_FUNCTION;
   TYPE_LENGTH (internal_fn_type) = sizeof (struct internal_function *);
   TYPE_NAME (internal_fn_type) = "<internal function>";
+
+  make_final_cleanup (value_history_cleanup, NULL);
 }
-- 
1.6.0.6

Re: [patch] [5/5] Types reference counting [value_history_cleanup]

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> There is now a fix to free anything at all.  When the function
Jan> was written there was an older implementation of type_decref
Jan> which was incorrectly freeing the zero-reference-count types
Jan> immediately (and not just on the garbage collection by
Jan> free_all_types).

Jan> gdb/
Jan> 2009-04-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	* value.c (value_history_cleanup): New function.
Jan> 	(_initialize_values): Setup a value_history_cleanup final call.

I don't generally like to add work on the exit path.

So, I think your initial comment was on target -- we should have some
kind of checking mode that enables this; and then do like GCC and
arrange to have it enabled in non-release builds.

Jan> +/* Sanity check for memory leaks and proper types reference counting.  */
Jan> +
Jan> +static void
Jan> +value_history_cleanup (void *unused)
Jan> +{

This should probably free values_in_python as well.

I don't think we have a convenient way to destroy all the types
referenced by Type objects, but perhaps we ought to add one for this.

Tom