Reference count struct dictionary.
authorJohn Darrington <john@darrington.wattle.id.au>
Sat, 29 Sep 2018 10:51:02 +0000 (12:51 +0200)
committerJohn Darrington <john@darrington.wattle.id.au>
Mon, 1 Oct 2018 18:29:19 +0000 (20:29 +0200)
This change:

* Adds a ref count to struct dictionary.
* Wraps dict_destroy in a ref-counted wrapper dict_unref.
* Replaces most calls to dict_destroy with dict_unref.
* Adds a new function dict_ref.
* Changes PsppireDict so take a reference to the struct dict it contains.
* Fixes some leaked dictionaries in PsppireImportAssistant.

27 files changed:
perl-module/PSPP.xs
src/data/any-reader.c
src/data/dataset-writer.c
src/data/dataset.c
src/data/dictionary.c
src/data/dictionary.h
src/data/gnumeric-reader.c
src/data/ods-reader.c
src/data/pc+-file-reader.c
src/data/por-file-reader.c
src/data/psql-reader.c
src/data/spreadsheet-reader.h
src/data/sys-file-reader.c
src/language/data-io/combine-files.c
src/language/data-io/data-list.c
src/language/data-io/get-data.c
src/language/data-io/get.c
src/language/data-io/matrix-data.c
src/language/data-io/save-translate.c
src/language/data-io/save.c
src/language/dictionary/modify-variables.c
src/language/dictionary/sys-file-info.c
src/language/stats/aggregate.c
src/language/stats/flip.c
src/ui/gui/psppire-dict.c
src/ui/gui/psppire-import-assistant.c
utilities/pspp-convert.c

index 74ea561170b00b3b16917aa8fe1c5a2c9fa24309..ea2907ffe9b68ca7a69a43291b26a9c57af42ba6 100644 (file)
@@ -274,7 +274,7 @@ CODE:
         free (input_format);
        }
      hmap_destroy (&dict->input_formats);
-     dict_destroy (dict->dict);
+     dict_unref (dict->dict);
      free (dict);
    }
 
index ff7f4ab6c923a4dd7bec8ea3e425e5523262b2bd..a77fa2793f59e8dc028d1367d5781093afca3d50 100644 (file)
@@ -222,7 +222,7 @@ static bool
 dataset_reader_close (struct any_reader *r_)
 {
   struct dataset_reader *r = dataset_reader_cast (r_);
-  dict_destroy (r->dict);
+  dict_unref (r->dict);
   casereader_destroy (r->reader);
   free (r);
 
index c810cf2a8f2bea2b07b53f51bedb16c085943fd8..5d14203d3d9354fa664eb19ffc42245f91e5d0a5 100644 (file)
@@ -115,7 +115,7 @@ dataset_writer_casewriter_destroy (struct casewriter *w UNUSED, void *writer_)
   else
     {
       casereader_destroy (reader);
-      dict_destroy (writer->dict);
+      dict_unref (writer->dict);
     }
 
   fh_unlock (writer->lock);
index 7a5a6a4a6a08a22749baf5340b9e71d709d66c51..c426cddb3c5662846865828d41650665481f6647 100644 (file)
@@ -199,7 +199,7 @@ dataset_destroy (struct dataset *ds)
     {
       dataset_set_session (ds, NULL);
       dataset_clear (ds);
-      dict_destroy (ds->dict);
+      dict_unref (ds->dict);
       caseinit_destroy (ds->caseinit);
       trns_chain_destroy (ds->permanent_trns_chain);
       dataset_transformations_changed__ (ds, false);
@@ -292,7 +292,7 @@ dataset_set_dict (struct dataset *ds, struct dictionary *dict)
 
   dataset_clear (ds);
 
-  dict_destroy (ds->dict);
+  dict_unref (ds->dict);
   ds->dict = dict;
   dict_set_change_callback (ds->dict, dict_callback, ds);
 }
@@ -765,7 +765,7 @@ proc_make_temporary_transformations_permanent (struct dataset *ds)
 
       ds->cur_trns_chain = ds->permanent_trns_chain;
 
-      dict_destroy (ds->permanent_dict);
+      dict_unref (ds->permanent_dict);
       ds->permanent_dict = NULL;
 
       return true;
@@ -782,7 +782,7 @@ proc_cancel_temporary_transformations (struct dataset *ds)
 {
   if (proc_in_temporary_transformations (ds))
     {
-      dict_destroy (ds->dict);
+      dict_unref (ds->dict);
       ds->dict = ds->permanent_dict;
       ds->permanent_dict = NULL;
 
index ff5f8ec027f16cbe3907ac5ab47acecb1e338425..d34bd9a358214e19353ad7793d9f66cc1311a3c3 100644 (file)
@@ -55,6 +55,7 @@
 /* A dictionary. */
 struct dictionary
   {
+    int ref_cnt;
     struct vardict_info *var;  /* Variables. */
     size_t var_cnt, var_cap;    /* Number of variables, capacity. */
     struct caseproto *proto;    /* Prototype for dictionary cases
@@ -179,10 +180,18 @@ dict_create (const char *encoding)
   d->names_must_be_ids = true;
   hmap_init (&d->name_map);
   attrset_init (&d->attributes);
+  d->ref_cnt = 1;
 
   return d;
 }
 
+struct dictionary *
+dict_ref (struct dictionary *s)
+{
+  s->ref_cnt++;
+  return s;
+}
+
 /* Creates and returns a (deep) copy of an existing
    dictionary.
 
@@ -219,7 +228,7 @@ dict_clone (const struct dictionary *s)
   d->split_cnt = s->split_cnt;
   if (d->split_cnt > 0)
     {
-      d->split = xnmalloc (d->split_cnt, sizeof *d->split);
+       d->split = xnmalloc (d->split_cnt, sizeof *d->split);
       for (i = 0; i < d->split_cnt; i++)
         d->split[i] = dict_lookup_var_assert (d, var_get_name (s->split[i]));
     }
@@ -288,23 +297,31 @@ dict_clear (struct dictionary *d)
 }
 
 /* Clears a dictionary and destroys it. */
+static void
+_dict_destroy (struct dictionary *d)
+{
+  /* In general, we don't want callbacks occurring, if the dictionary
+     is being destroyed */
+  d->callbacks  = NULL ;
+
+  dict_clear (d);
+  string_array_destroy (&d->documents);
+  hmap_destroy (&d->name_map);
+  attrset_destroy (&d->attributes);
+  dict_clear_mrsets (d);
+  free (d->encoding);
+  free (d);
+}
+
 void
-dict_destroy (struct dictionary *d)
+dict_unref (struct dictionary *d)
 {
-  if (d != NULL)
-    {
-      /* In general, we don't want callbacks occurring, if the dictionary
-        is being destroyed */
-      d->callbacks  = NULL ;
-
-      dict_clear (d);
-      string_array_destroy (&d->documents);
-      hmap_destroy (&d->name_map);
-      attrset_destroy (&d->attributes);
-      dict_clear_mrsets (d);
-      free (d->encoding);
-      free (d);
-    }
+  if (d == NULL)
+    return;
+  d->ref_cnt--;
+  assert (d->ref_cnt >= 0);
+  if (d->ref_cnt == 0)
+    _dict_destroy (d);
 }
 
 /* Returns the number of variables in D. */
@@ -1703,7 +1720,7 @@ dict_destroy_internal_var (struct variable *var)
          valgrind --leak-check --show-reachable won't show internal_dict. */
       if (dict_get_var_cnt (internal_dict) == 0)
         {
-          dict_destroy (internal_dict);
+          dict_unref (internal_dict);
           internal_dict = NULL;
         }
     }
index a4be0fd44898f01b4ac4ff76fea663ce51942b6c..8a760081d3b935241f506ac1ffed5956417a87eb 100644 (file)
@@ -27,12 +27,13 @@ struct ccase;
 
 /* Creating dictionaries. */
 struct dictionary *dict_create (const char *encoding);
-struct dictionary *dict_clone (const struct dictionary *);
+struct dictionary *dict_clone (const struct dictionary *) WARN_UNUSED_RESULT;
+struct dictionary *dict_ref (struct dictionary *s) WARN_UNUSED_RESULT;
 
 
 /* Clearing and destroying dictionaries. */
 void dict_clear (struct dictionary *);
-void dict_destroy (struct dictionary *);
+void dict_unref (struct dictionary *);
 
 /* Common ways to access variables. */
 struct variable *dict_lookup_var (const struct dictionary *, const char *);
index 589bdbe8c5d02119e1ea2046b8e89248eb83b387..5fee6919c0a51891f5240328606086cf331eb15b 100644 (file)
@@ -174,7 +174,7 @@ gnumeric_unref (struct spreadsheet *s)
       free (r->sheets);
       state_data_destroy (&r->msd);
 
-      dict_destroy (r->dict);
+      dict_unref (r->dict);
 
       free (s->file_name);
 
index ce9310a230b23385891eaed984f8ce929bdc7819..212598637ac87b930b5564fd002a499258a0af03 100644 (file)
@@ -153,7 +153,7 @@ ods_unref (struct spreadsheet *s)
          xmlFree (r->sheets[i].name);
        }
 
-      dict_destroy (r->dict);
+      dict_unref (r->dict);
 
       zip_reader_destroy (r->zreader);
       free (r->sheets);
index cc80cd723b1d142876967f4225c949a715b6aa04..979be50c47aacef160540c46179d00e1fc10efa8 100644 (file)
@@ -461,7 +461,7 @@ pcp_decode (struct any_reader *r_, const char *encoding,
 
 error:
   pcp_close (&r->any_reader);
-  dict_destroy (dict);
+  dict_unref (dict);
   *dictp = NULL;
   return NULL;
 }
index 15a3b7902e45b59f1548299e412482044897fa9c..8745cfc68fb5acf7e1cf02d4a7c7366c2504c132 100644 (file)
@@ -166,7 +166,7 @@ pfm_close (struct any_reader *r_)
   struct pfm_reader *r = pfm_reader_cast (r_);
   bool ok;
 
-  dict_destroy (r->dict);
+  dict_unref (r->dict);
   any_read_info_destroy (&r->info);
   if (r->file)
     {
index d28085f1f4a5ab13307739a89876c0236b8a8c5e..4a49ba0cf92c413c551811b1ce1a87dd9e64c062 100644 (file)
@@ -544,7 +544,7 @@ psql_open_reader (struct psql_read_info *info, struct dictionary **dict)
      &psql_casereader_class, r);
 
  error:
-  dict_destroy (*dict);
+  dict_unref (*dict);
 
   psql_casereader_destroy (NULL, r);
   return NULL;
index 6b026b01cdae791365d6af3a3629e63f69f05244..efba6f369fbf3577511aa01f422c64d241893db4 100644 (file)
@@ -69,8 +69,8 @@ struct spreadsheet
   int n_sheets;
 
   /* The dictionary for client's reference.
-     Client must clone if it needs a permanent or modifiable copy. */
-  const struct dictionary *dict;
+     Client must ref or clone it if it needs a permanent or modifiable copy. */
+  struct dictionary *dict;
 
   int ref_cnt;
 };
index de5653a3542294206fbc319c0366525a6b89da97..20ecf048f907e2fbc6bf9a317fb8884c00ba4d53 100644 (file)
@@ -891,7 +891,7 @@ sfm_decode (struct any_reader *r_, const char *encoding,
 
 error:
   sfm_close (r_);
-  dict_destroy (dict);
+  dict_unref (dict);
   *dictp = NULL;
   return NULL;
 }
index 6d9ed5d43b0d628ca41390121837eaf9c2a38f7a..b4eac56a6b4a4d270f19b2b47c4fc3b21ececaa5 100644 (file)
@@ -644,7 +644,7 @@ close_all_comb_files (struct comb_proc *proc)
       subcase_destroy (&file->dst);
       free (file->mv);
       fh_unref (file->handle);
-      dict_destroy (file->dict);
+      dict_unref (file->dict);
       casereader_destroy (file->reader);
       case_unref (file->data);
       free (file->in_name);
@@ -659,7 +659,7 @@ static void
 free_comb_proc (struct comb_proc *proc)
 {
   close_all_comb_files (proc);
-  dict_destroy (proc->dict);
+  dict_unref (proc->dict);
   casewriter_destroy (proc->output);
   case_matcher_destroy (proc->matcher);
   if (proc->prev_BY)
index 0b98f40a07e2a2e08d6e54ce0a944473f68c1738..87f100f823f9f7a998fe58e2002eb7eb08e7359d 100644 (file)
@@ -316,7 +316,7 @@ cmd_data_list (struct lexer *lexer, struct dataset *ds)
  error:
   data_parser_destroy (parser);
   if (!in_input_program ())
-    dict_destroy (dict);
+    dict_unref (dict);
   fh_unref (fh);
   free (encoding);
   return CMD_CASCADING_FAILURE;
index dff449851bd91ff8a5bc3b3b18a26b196242f603..8a58be434c71a6b61de8497bb255458fd3d3daf5 100644 (file)
@@ -686,7 +686,7 @@ parse_get_txt (struct lexer *lexer, struct dataset *ds)
 
  error:
   data_parser_destroy (parser);
-  dict_destroy (dict);
+  dict_unref (dict);
   fh_unref (fh);
   free (name);
   free (encoding);
index 2440bd900b9d2ee01fcfac78342f70f88004c36b..6e99187ba11c5a311d137b2f6be1dbc0990ae3ed 100644 (file)
@@ -161,7 +161,7 @@ parse_read_command (struct lexer *lexer, struct dataset *ds,
   fh_unref (fh);
   casereader_destroy (reader);
   if (dict != NULL)
-    dict_destroy (dict);
+    dict_unref (dict);
   free (encoding);
   return CMD_CASCADING_FAILURE;
 }
index 6beb19b1fb8ca71968ca3f29741218fa17e5b875..97d14739b3a56ab2b8ef7f1cc38ad137ddf911d4 100644 (file)
@@ -573,7 +573,7 @@ cmd_matrix (struct lexer *lexer, struct dataset *ds)
  error:
   data_parser_destroy (parser);
   if (!in_input_program ())
-    dict_destroy (dict);
+    dict_unref (dict);
   fh_unref (fh);
   free (encoding);
   free (mformat.split_vars);
index e68b4133bb6419f466c3aadc9745e86868b52453..e2e787347f9bb6f55d39735c084df8e85737925f 100644 (file)
@@ -279,7 +279,7 @@ cmd_save_translate (struct lexer *lexer, struct dataset *ds)
   case_map_stage_destroy (stage);
   if (map != NULL)
     writer = case_map_create_output_translator (map, writer);
-  dict_destroy (dict);
+  dict_unref (dict);
 
   casereader_transfer (proc_open_filtering (ds, !retain_unselected), writer);
   ok = casewriter_destroy (writer);
@@ -290,7 +290,7 @@ cmd_save_translate (struct lexer *lexer, struct dataset *ds)
 error:
   case_map_stage_destroy (stage);
   fh_unref (handle);
-  dict_destroy (dict);
+  dict_unref (dict);
   case_map_destroy (map);
   return CMD_FAILURE;
 }
index cec878766ad07b2375c37afe5c044583ef8d834a..be746742f380403036c021b7a626d2b21bc4bbca 100644 (file)
@@ -342,7 +342,7 @@ parse_write_command (struct lexer *lexer, struct dataset *ds,
   case_map_stage_destroy (stage);
   if (map != NULL)
     writer = case_map_create_output_translator (map, writer);
-  dict_destroy (dict);
+  dict_unref (dict);
 
   fh_unref (handle);
   fh_unref (metadata);
@@ -353,7 +353,7 @@ parse_write_command (struct lexer *lexer, struct dataset *ds,
   fh_unref (handle);
   fh_unref (metadata);
   casewriter_destroy (writer);
-  dict_destroy (dict);
+  dict_unref (dict);
   case_map_destroy (map);
   return NULL;
 }
index 70c700084fdf765450ce59a2d758f1a7da609ffa..d94c779298f3a6832fe5323d916e680e39d3b055 100644 (file)
@@ -296,7 +296,7 @@ cmd_modify_vars (struct lexer *lexer, struct dataset *ds)
             {
               /* FIXME: display new dictionary. */
             }
-          dict_destroy (temp);
+          dict_unref (temp);
        }
       else
        {
index 82a6fea780d4da8144dece3e2189d062beaad268..ead50a6c6a0258f7951041658b86ae2a172ba270 100644 (file)
@@ -250,7 +250,7 @@ cmd_sysfile_info (struct lexer *lexer, struct dataset *ds UNUSED)
 
   table_item_submit (table_item_create (table, NULL /* XXX */, NULL));
 
-  dict_destroy (d);
+  dict_unref (d);
 
   fh_unref (h);
   free (encoding);
index 03117a5e6135ce4b30ac1eee01e0529654c450af..7a2c986bf04282051a7854e1c671a097daf35565 100644 (file)
@@ -733,7 +733,7 @@ agr_destroy (struct agr_proc *agr)
       free (iter);
     }
   if (agr->dict != NULL)
-    dict_destroy (agr->dict);
+    dict_unref (agr->dict);
 }
 \f
 /* Execution. */
index 7c28d3a1da0007fe1da2a54c1dd03d221b39f5a9..2ed7278d6b4129dc97ece818ca35849fd951e04b 100644 (file)
@@ -231,7 +231,7 @@ cmd_flip (struct lexer *lexer, struct dataset *ds)
   return CMD_SUCCESS;
 
  error:
-  dict_destroy (new_dict);
+  dict_unref (new_dict);
   destroy_flip_pgm (flip);
   return CMD_CASCADING_FAILURE;
 }
index 0afb889037788e74b577581844d0c5306065bbcf..63ce9d33080c9f1bd255b34315f438edbd2929ac 100644 (file)
@@ -288,6 +288,7 @@ psppire_dict_dispose (GObject *object)
   PsppireDict *d = PSPPIRE_DICT (object);
 
   dict_set_callbacks (d->dict, NULL, NULL);
+  dict_unref (d->dict);
 
   G_OBJECT_CLASS (parent_class)->dispose (object);
 }
@@ -367,7 +368,7 @@ PsppireDict*
 psppire_dict_new_from_dict (struct dictionary *d)
 {
   PsppireDict *new_dict = g_object_new (PSPPIRE_TYPE_DICT, NULL);
-  new_dict->dict = d;
+  new_dict->dict = dict_ref (d);
 
   dict_set_callbacks (new_dict->dict, &gui_callbacks, new_dict);
 
@@ -380,10 +381,13 @@ psppire_dict_replace_dictionary (PsppireDict *dict, struct dictionary *d)
 {
   struct variable *var =  dict_get_weight (d);
 
+  struct dictionary *old_dict = dict->dict;
+
   guint old_n = dict_get_var_cnt (dict->dict);
   guint new_n = dict_get_var_cnt (d);
 
-  dict->dict = d;
+  dict->dict = dict_ref (d);
+  dict_unref (old_dict);
 
   weight_changed_callback (d, var ? var_get_dict_index (var) : -1, dict);
 
index c4110ad3d624b6eeaee6f73325c246edc59fa2e4..e376bacc7e667cf3ae9076d071c193fa940936ea 100644 (file)
@@ -127,6 +127,8 @@ psppire_import_assistant_finalize (GObject *object)
 
   ds_destroy (&ia->quotes);
 
+  dict_unref (ia->dict);
+
   g_object_unref (ia->builder);
 
   ia->response = -1;
@@ -1274,7 +1276,7 @@ prepare_formats_page (PsppireImportAssistant *ia)
 
       struct casereader *reader = spreadsheet_make_reader (ia->spreadsheet, &opts);
 
-      PsppireDict *dict = psppire_dict_new_from_dict (dict_clone (ia->spreadsheet->dict));
+      PsppireDict *dict = psppire_dict_new_from_dict (ia->spreadsheet->dict);
       PsppireDataStore *store = psppire_data_store_new (dict);
       psppire_data_store_set_reader (store, reader);
       g_object_set (ia->data_sheet, "data-model", store, NULL);
index f21e5bdb8b907cb51a25a331d9311f436fc743e5..217576ae7bcac55012f04dd7d1730ef1ede8f65b 100644 (file)
@@ -223,7 +223,7 @@ main (int argc, char *argv[])
     error (1, 0, _("%s: error writing output file"), output_filename);
 
 exit:
-  dict_destroy (dict);
+  dict_unref (dict);
   fh_unref (output_fh);
   fh_unref (input_fh);
   fh_done ();
@@ -232,7 +232,7 @@ exit:
   return 0;
 
 error:
-  dict_destroy (dict);
+  dict_unref (dict);
   fh_unref (output_fh);
   fh_unref (input_fh);
   fh_done ();