On Sun, Sep 23, 2018 at 11:00:17PM -0700, Ben Pfaff wrote:
It seems like refcounting the dictionary will require a lot of care to
find all the places where a dictionary gets modified to un-share. I
seem to recall that the variables in a dictionary have a reference
upward to the dictionary, too.
I'm not saying that refcounting the dictionary is the wrong choice but
it needs to be done carefully.
This version seems to fix all the memory error problems, and so far as
I'm aware doesn't introduce any new issues.
J'
diff --git a/perl-module/PSPP.xs b/perl-module/PSPP.xs
index 74ea5611..ea2907ff 100644
--- a/perl-module/PSPP.xs
+++ b/perl-module/PSPP.xs
@@ -274,7 +274,7 @@ CODE:
free (input_format);
}
hmap_destroy (&dict->input_formats);
- dict_destroy (dict->dict);
+ dict_unref (dict->dict);
free (dict);
}
diff --git a/src/data/any-reader.c b/src/data/any-reader.c
index ff7f4ab6..a77fa279 100644
--- a/src/data/any-reader.c
+++ b/src/data/any-reader.c
@@ -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);
diff --git a/src/data/dataset-writer.c b/src/data/dataset-writer.c
index c810cf2a..5d14203d 100644
--- a/src/data/dataset-writer.c
+++ b/src/data/dataset-writer.c
@@ -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);
diff --git a/src/data/dataset.c b/src/data/dataset.c
index 7a5a6a4a..c426cddb 100644
--- a/src/data/dataset.c
+++ b/src/data/dataset.c
@@ -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;
diff --git a/src/data/dictionary.c b/src/data/dictionary.c
index ff5f8ec0..f2163c3b 100644
--- a/src/data/dictionary.c
+++ b/src/data/dictionary.c
@@ -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,30 @@ 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--;
+ if (d->ref_cnt == 0)
+ _dict_destroy (d);
}
/* Returns the number of variables in D. */
@@ -1703,7 +1719,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;
}
}
diff --git a/src/data/dictionary.h b/src/data/dictionary.h
index a4be0fd4..8a760081 100644
--- a/src/data/dictionary.h
+++ b/src/data/dictionary.h
@@ -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 *);
diff --git a/src/data/gnumeric-reader.c b/src/data/gnumeric-reader.c
index 589bdbe8..5fee6919 100644
--- a/src/data/gnumeric-reader.c
+++ b/src/data/gnumeric-reader.c
@@ -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);
diff --git a/src/data/ods-reader.c b/src/data/ods-reader.c
index ce9310a2..21259863 100644
--- a/src/data/ods-reader.c
+++ b/src/data/ods-reader.c
@@ -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);
diff --git a/src/data/pc+-file-reader.c b/src/data/pc+-file-reader.c
index cc80cd72..979be50c 100644
--- a/src/data/pc+-file-reader.c
+++ b/src/data/pc+-file-reader.c
@@ -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;
}
diff --git a/src/data/por-file-reader.c b/src/data/por-file-reader.c
index 15a3b790..8745cfc6 100644
--- a/src/data/por-file-reader.c
+++ b/src/data/por-file-reader.c
@@ -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)
{
diff --git a/src/data/psql-reader.c b/src/data/psql-reader.c
index d28085f1..4a49ba0c 100644
--- a/src/data/psql-reader.c
+++ b/src/data/psql-reader.c
@@ -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;
diff --git a/src/data/sys-file-reader.c b/src/data/sys-file-reader.c
index 1b2d6c21..e04b22d8 100644
--- a/src/data/sys-file-reader.c
+++ b/src/data/sys-file-reader.c
@@ -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;
}
diff --git a/src/language/data-io/combine-files.c b/src/language/data-io/combine-files.c
index 6d9ed5d4..b4eac56a 100644
--- a/src/language/data-io/combine-files.c
+++ b/src/language/data-io/combine-files.c
@@ -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)
diff --git a/src/language/data-io/data-list.c b/src/language/data-io/data-list.c
index 0b98f40a..87f100f8 100644
--- a/src/language/data-io/data-list.c
+++ b/src/language/data-io/data-list.c
@@ -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;
diff --git a/src/language/data-io/get-data.c b/src/language/data-io/get-data.c
index dff44985..8a58be43 100644
--- a/src/language/data-io/get-data.c
+++ b/src/language/data-io/get-data.c
@@ -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);
diff --git a/src/language/data-io/get.c b/src/language/data-io/get.c
index 2440bd90..6e99187b 100644
--- a/src/language/data-io/get.c
+++ b/src/language/data-io/get.c
@@ -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;
}
diff --git a/src/language/data-io/matrix-data.c b/src/language/data-io/matrix-data.c
index 6beb19b1..97d14739 100644
--- a/src/language/data-io/matrix-data.c
+++ b/src/language/data-io/matrix-data.c
@@ -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);
diff --git a/src/language/data-io/save-translate.c b/src/language/data-io/save-translate.c
index e68b4133..e2e78734 100644
--- a/src/language/data-io/save-translate.c
+++ b/src/language/data-io/save-translate.c
@@ -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;
}
diff --git a/src/language/data-io/save.c b/src/language/data-io/save.c
index cec87876..be746742 100644
--- a/src/language/data-io/save.c
+++ b/src/language/data-io/save.c
@@ -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;
}
diff --git a/src/language/dictionary/modify-variables.c b/src/language/dictionary/modify-variables.c
index 70c70008..d94c7792 100644
--- a/src/language/dictionary/modify-variables.c
+++ b/src/language/dictionary/modify-variables.c
@@ -296,7 +296,7 @@ cmd_modify_vars (struct lexer *lexer, struct dataset *ds)
{
/* FIXME: display new dictionary. */
}
- dict_destroy (temp);
+ dict_unref (temp);
}
else
{
diff --git a/src/language/dictionary/sys-file-info.c b/src/language/dictionary/sys-file-info.c
index 82a6fea7..ead50a6c 100644
--- a/src/language/dictionary/sys-file-info.c
+++ b/src/language/dictionary/sys-file-info.c
@@ -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);
diff --git a/src/language/stats/aggregate.c b/src/language/stats/aggregate.c
index 03117a5e..7a2c986b 100644
--- a/src/language/stats/aggregate.c
+++ b/src/language/stats/aggregate.c
@@ -733,7 +733,7 @@ agr_destroy (struct agr_proc *agr)
free (iter);
}
if (agr->dict != NULL)
- dict_destroy (agr->dict);
+ dict_unref (agr->dict);
}
/* Execution. */
diff --git a/src/language/stats/flip.c b/src/language/stats/flip.c
index 7c28d3a1..2ed7278d 100644
--- a/src/language/stats/flip.c
+++ b/src/language/stats/flip.c
@@ -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;
}
diff --git a/src/ui/gui/psppire-dict.c b/src/ui/gui/psppire-dict.c
index eecf81f6..1a269e07 100644
--- a/src/ui/gui/psppire-dict.c
+++ b/src/ui/gui/psppire-dict.c
@@ -288,7 +288,7 @@ psppire_dict_dispose (GObject *object)
PsppireDict *d = PSPPIRE_DICT (object);
dict_set_callbacks (d->dict, NULL, NULL);
- dict_destroy (d->dict);
+ dict_unref (d->dict);
G_OBJECT_CLASS (parent_class)->dispose (object);
}
@@ -368,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 = dict_clone (d);
+ new_dict->dict = dict_ref (d);
dict_set_callbacks (new_dict->dict, &gui_callbacks, new_dict);
@@ -381,13 +381,11 @@ psppire_dict_replace_dictionary (PsppireDict *dict, struct dictionary *d)
{
struct variable *var = dict_get_weight (d);
- assert (dict->dict != d);
-
guint old_n = dict_get_var_cnt (dict->dict);
guint new_n = dict_get_var_cnt (d);
- dict_destroy (dict->dict);
- dict->dict = dict_clone (d);
+ dict_unref (dict->dict);
+ dict->dict = dict_ref (d);
weight_changed_callback (d, var ? var_get_dict_index (var) : -1, dict);
diff --git a/src/ui/gui/psppire-import-assistant.c b/src/ui/gui/psppire-import-assistant.c
index 3db3d503..a9a85704 100644
--- a/src/ui/gui/psppire-import-assistant.c
+++ b/src/ui/gui/psppire-import-assistant.c
@@ -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;
diff --git a/utilities/pspp-convert.c b/utilities/pspp-convert.c
index f21e5bdb..217576ae 100644
--- a/utilities/pspp-convert.c
+++ b/utilities/pspp-convert.c
@@ -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 ();
_______________________________________________
pspp-dev mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/pspp-dev
