Re: Another bug in psppsheet branch

[Ben Pfaff]

John Darrington <address@hidden> writes:

> I noticed the following problem using the psppsheet branch as of
> d4ae90b2fe74d2d1427afad35d32c9e5584211ed
>
> 1. Prepare a .sav file (call it x.sav) with some data.
>
> 2. Start psppire
>
> 3. File | Open and load x.sav (dataset1)
>
> 4. File | New | Data (a new data window will appear: dataset2)
>
> 5. Using the window manager close the window for dataset1
>
> 6. In dataset2:  File | Open and load x.sav
>
>  A lot of Gtk-Criticals appear and the program quickly crashes.

Here's a partial fix.  What do you think?

--8<--------------------------cut here-------------------------->8--

From: Ben Pfaff <address@hidden>
Date: Sat, 7 Jul 2012 13:00:19 -0700
Subject: [PATCH] psppire-dict: Make PsppireDict not own its "struct dictionary".

I see two places where PsppireDict are created.  The first is in
psppire-data-window.c.  In this case, the PsppireDataWindow is
borrowing a dictionary owned by a "struct dataset" for use in its
PsppireDict.  When it destroys the PsppireDict, therefore, the
dictionary must not be destroyed because the dataset still owns it.
However, this is not what actually happens, and doing the
following:

1. Start psppire
2. File | Open and load x.sav (dataset1)
3. File | New | Data (a new data window will appear: dataset2)
4. Using the window manager close the window for dataset1
5. In dataset2:  File | Open and load x.sav

or similar will cause a use-after-free error.

The second use is in text-data-import-dialog.c.  This code does own
the struct dictionary, but it can simply free it itself at the same
time as the PsppireDict.

There is still some underlying issue with the above scenario,
because it still reports a GtkCritical, but valgrind no longer
reports a use-after-free error.

Reported by John Darrington.
---
 src/ui/gui/psppire-dict.c            |   18 +++---------------
 src/ui/gui/text-data-import-dialog.c |    2 +-
 2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/ui/gui/psppire-dict.c b/src/ui/gui/psppire-dict.c
index 312220f..343fbd0 100644
--- a/src/ui/gui/psppire-dict.c
+++ b/src/ui/gui/psppire-dict.c
@@ -56,7 +56,6 @@ enum  {
 /* --- prototypes --- */
 static void psppire_dict_class_init    (PsppireDictClass       *class);
 static void psppire_dict_init  (PsppireDict            *dict);
-static void psppire_dict_finalize      (GObject                *object);
 
 static void dictionary_tree_model_init (GtkTreeModelIface *iface);
 
@@ -111,12 +110,8 @@ psppire_dict_get_type (void)
 static void
 psppire_dict_class_init (PsppireDictClass *class)
 {
-  GObjectClass *object_class = G_OBJECT_CLASS (class);
-
   parent_class = g_type_class_peek_parent (class);
 
-  object_class->finalize = psppire_dict_finalize;
-
   signals [BACKEND_CHANGED] =
     g_signal_new ("backend-changed",
                  G_TYPE_FROM_CLASS (class),
@@ -226,16 +221,6 @@ psppire_dict_class_init (PsppireDictClass *class)
                  0);
 }
 
-static void
-psppire_dict_finalize (GObject *object)
-{
-  PsppireDict *d = PSPPIRE_DICT (object);
-
-  dict_destroy (d->dict);
-
-  G_OBJECT_CLASS (parent_class)->finalize (object);
-}
-
 /* Pass on callbacks from src/data/dictionary, as
    signals in the Gtk library */
 static void
@@ -317,6 +302,9 @@ psppire_dict_init (PsppireDict *psppire_dict)
  * @returns: a new #PsppireDict object
  *
  * Creates a new #PsppireDict.
+ *
+ * A #PsppireDict does not own its dictionary, that is, when the #PsppireDict
+ * is destroyed it does not destroy the underlying #struct dictionary.
  */
 PsppireDict*
 psppire_dict_new_from_dict (struct dictionary *d)
diff --git a/src/ui/gui/text-data-import-dialog.c 
b/src/ui/gui/text-data-import-dialog.c
index b5a50cb..0abeb56 100644
--- a/src/ui/gui/text-data-import-dialog.c
+++ b/src/ui/gui/text-data-import-dialog.c
@@ -1625,7 +1625,7 @@ destroy_formats_page (struct import_assistant *ia)
 
   if (p->psppire_dict != NULL)
     {
-      /* This destroys p->dict also. */
+      dict_destroy (p->psppire_dict->dict);
       g_object_unref (p->psppire_dict);
     }
   clear_modified_vars (ia);
-- 
1.7.2.5