psppire-dict: Make PsppireDict not own its "struct dictionary".
authorBen Pfaff <blp@cs.stanford.edu>
Sat, 7 Jul 2012 20:00:19 +0000 (13:00 -0700)
committerBen Pfaff <blp@cs.stanford.edu>
Mon, 9 Jul 2012 05:15:13 +0000 (22:15 -0700)
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
src/ui/gui/text-data-import-dialog.c

index 312220f9de3ac23263d0e242c10bc46bef00cf5f..e6291b69c465535ee94434908070a9b78772d031 100644 (file)
@@ -56,7 +56,7 @@ 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 psppire_dict_dispose       (GObject                *object);
 
 static void dictionary_tree_model_init (GtkTreeModelIface *iface);
 
@@ -115,7 +115,7 @@ psppire_dict_class_init (PsppireDictClass *class)
 
   parent_class = g_type_class_peek_parent (class);
 
-  object_class->finalize = psppire_dict_finalize;
+  object_class->dispose = psppire_dict_dispose;
 
   signals [BACKEND_CHANGED] =
     g_signal_new ("backend-changed",
@@ -227,13 +227,13 @@ psppire_dict_class_init (PsppireDictClass *class)
 }
 
 static void
-psppire_dict_finalize (GObject *object)
+psppire_dict_dispose (GObject *object)
 {
   PsppireDict *d = PSPPIRE_DICT (object);
 
-  dict_destroy (d->dict);
+  dict_set_callbacks (d->dict, NULL, NULL);
 
-  G_OBJECT_CLASS (parent_class)->finalize (object);
+  G_OBJECT_CLASS (parent_class)->dispose (object);
 }
 
 /* Pass on callbacks from src/data/dictionary, as
index c3d5ada9bd535bc7756582e47d8ad0f609d67a13..907dd7ab4cb683ebe4f61a3fcc842ec35ec64e48 100644 (file)
@@ -1626,7 +1626,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);