From 8d6bfdd2a100bf8166b3b0b3f006d46f3e7a59e9 Mon Sep 17 00:00:00 2001 From: John Darrington Date: Sat, 29 Sep 2018 12:51:02 +0200 Subject: [PATCH] Reference count struct dictionary. 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. --- perl-module/PSPP.xs | 2 +- src/data/any-reader.c | 2 +- src/data/dataset-writer.c | 2 +- src/data/dataset.c | 8 ++-- src/data/dictionary.c | 51 ++++++++++++++-------- src/data/dictionary.h | 5 ++- src/data/gnumeric-reader.c | 2 +- src/data/ods-reader.c | 2 +- src/data/pc+-file-reader.c | 2 +- src/data/por-file-reader.c | 2 +- src/data/psql-reader.c | 2 +- src/data/spreadsheet-reader.h | 4 +- src/data/sys-file-reader.c | 2 +- src/language/data-io/combine-files.c | 4 +- src/language/data-io/data-list.c | 2 +- src/language/data-io/get-data.c | 2 +- src/language/data-io/get.c | 2 +- src/language/data-io/matrix-data.c | 2 +- src/language/data-io/save-translate.c | 4 +- src/language/data-io/save.c | 4 +- src/language/dictionary/modify-variables.c | 2 +- src/language/dictionary/sys-file-info.c | 2 +- src/language/stats/aggregate.c | 2 +- src/language/stats/flip.c | 2 +- src/ui/gui/psppire-dict.c | 8 +++- src/ui/gui/psppire-import-assistant.c | 4 +- utilities/pspp-convert.c | 4 +- 27 files changed, 77 insertions(+), 53 deletions(-) diff --git a/perl-module/PSPP.xs b/perl-module/PSPP.xs index 74ea561170..ea2907ffe9 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 ff7f4ab6c9..a77fa2793f 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 c810cf2a8f..5d14203d3d 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 7a5a6a4a6a..c426cddb3c 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 ff5f8ec027..d34bd9a358 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,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; } } diff --git a/src/data/dictionary.h b/src/data/dictionary.h index a4be0fd448..8a760081d3 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 589bdbe8c5..5fee6919c0 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 ce9310a230..212598637a 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 cc80cd723b..979be50c47 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 15a3b7902e..8745cfc68f 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 d28085f1f4..4a49ba0cf9 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/spreadsheet-reader.h b/src/data/spreadsheet-reader.h index 6b026b01cd..efba6f369f 100644 --- a/src/data/spreadsheet-reader.h +++ b/src/data/spreadsheet-reader.h @@ -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; }; diff --git a/src/data/sys-file-reader.c b/src/data/sys-file-reader.c index de5653a354..20ecf048f9 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 6d9ed5d43b..b4eac56a6b 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 0b98f40a07..87f100f823 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 dff449851b..8a58be434c 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 2440bd900b..6e99187ba1 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 6beb19b1fb..97d14739b3 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 e68b4133bb..e2e787347f 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 cec878766a..be746742f3 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 70c700084f..d94c779298 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 82a6fea780..ead50a6c6a 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 03117a5e61..7a2c986bf0 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 7c28d3a1da..2ed7278d6b 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 0afb889037..63ce9d3308 100644 --- a/src/ui/gui/psppire-dict.c +++ b/src/ui/gui/psppire-dict.c @@ -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); diff --git a/src/ui/gui/psppire-import-assistant.c b/src/ui/gui/psppire-import-assistant.c index c4110ad3d6..e376bacc7e 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; @@ -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); diff --git a/utilities/pspp-convert.c b/utilities/pspp-convert.c index f21e5bdb8b..217576ae7b 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 (); -- 2.30.2