From c2a6f1e9111ce0b2105ef8f4c4446a6ccc3d6672 Mon Sep 17 00:00:00 2001 From: Robert Fewell <14uBobIT@gmail.com> Date: Tue, 13 Jul 2021 15:05:59 +0100 Subject: [PATCH 1/9] Free TaxTable and BillTerms on book close --- libgnucash/engine/gncBillTerm.c | 14 ++++++++++++++ libgnucash/engine/gncTaxTable.c | 13 +++++++++++++ 2 files changed, 27 insertions(+) diff --git a/libgnucash/engine/gncBillTerm.c b/libgnucash/engine/gncBillTerm.c index fb6f2bb15c..844a2e398b 100644 --- a/libgnucash/engine/gncBillTerm.c +++ b/libgnucash/engine/gncBillTerm.c @@ -838,14 +838,28 @@ static void _gncBillTermCreate (QofBook *book) qof_book_set_data (book, _GNC_MOD_NAME, bi); } + +static void +destroy_billterm_on_book_close (QofInstance *ent, gpointer data) +{ + GncBillTerm *term = GNC_BILLTERM(ent); + + gncBillTermBeginEdit (term); + gncBillTermDestroy (term); +} + static void _gncBillTermDestroy (QofBook *book) { struct _book_info *bi; + QofCollection *col; if (!book) return; bi = qof_book_get_data (book, _GNC_MOD_NAME); + col = qof_book_get_collection (book, GNC_ID_BILLTERM); + qof_collection_foreach (col, destroy_billterm_on_book_close, NULL); + g_list_free (bi->terms); g_free (bi); } diff --git a/libgnucash/engine/gncTaxTable.c b/libgnucash/engine/gncTaxTable.c index 3cd689db5c..23f195a648 100644 --- a/libgnucash/engine/gncTaxTable.c +++ b/libgnucash/engine/gncTaxTable.c @@ -1015,14 +1015,27 @@ static void _gncTaxTableCreate (QofBook *book) qof_book_set_data (book, _GNC_MOD_NAME, bi); } +static void +destroy_taxtable_on_book_close (QofInstance *ent, gpointer data) +{ + GncTaxTable *table = GNC_TAXTABLE(ent); + + gncTaxTableBeginEdit (table); + gncTaxTableDestroy (table); +} + static void _gncTaxTableDestroy (QofBook *book) { struct _book_info *bi; + QofCollection *col; if (!book) return; bi = qof_book_get_data (book, _GNC_MOD_NAME); + col = qof_book_get_collection (book, GNC_ID_TAXTABLE); + qof_collection_foreach (col, destroy_taxtable_on_book_close, NULL); + g_list_free (bi->tables); g_free (bi); } From b421b3d2af7dfaecbf2c6befbdfe374c16ad123a Mon Sep 17 00:00:00 2001 From: Robert Fewell <14uBobIT@gmail.com> Date: Tue, 13 Jul 2021 15:09:11 +0100 Subject: [PATCH 2/9] Free Jobs for Customers and Vendors on book close --- libgnucash/engine/gncCustomer.c | 2 ++ libgnucash/engine/gncJob.c | 12 ++++++++++++ libgnucash/engine/gncJob.h | 1 + libgnucash/engine/gncVendor.c | 2 ++ 4 files changed, 17 insertions(+) diff --git a/libgnucash/engine/gncCustomer.c b/libgnucash/engine/gncCustomer.c index ddb03aee04..2ce39c8cce 100644 --- a/libgnucash/engine/gncCustomer.c +++ b/libgnucash/engine/gncCustomer.c @@ -353,6 +353,8 @@ static void gncCustomerFree (GncCustomer *cust) gncAddressDestroy (cust->addr); gncAddressBeginEdit (cust->shipaddr); gncAddressDestroy (cust->shipaddr); + + gncJobFreeList (cust->jobs); g_list_free (cust->jobs); g_free (cust->balance); diff --git a/libgnucash/engine/gncJob.c b/libgnucash/engine/gncJob.c index 46fb843d61..8d9e42297d 100644 --- a/libgnucash/engine/gncJob.c +++ b/libgnucash/engine/gncJob.c @@ -231,6 +231,18 @@ GncJob *gncJobCreate (QofBook *book) return job; } +static void free_job_list (GncJob *job) +{ + gncJobBeginEdit (job); + gncJobDestroy (job); +} + +void gncJobFreeList (GList *jobs) +{ + GList *job_list = g_list_copy (jobs); + g_list_free_full (job_list, (GDestroyNotify)free_job_list); +} + void gncJobDestroy (GncJob *job) { if (!job) return; diff --git a/libgnucash/engine/gncJob.h b/libgnucash/engine/gncJob.h index b40cdbcab4..e68585f9e3 100644 --- a/libgnucash/engine/gncJob.h +++ b/libgnucash/engine/gncJob.h @@ -57,6 +57,7 @@ GType gnc_job_get_type(void); GncJob *gncJobCreate (QofBook *book); void gncJobDestroy (GncJob *job); +void gncJobFreeList (GList *jobs); /** \name Set Functions @{ diff --git a/libgnucash/engine/gncVendor.c b/libgnucash/engine/gncVendor.c index 67af18c61e..50695de27d 100644 --- a/libgnucash/engine/gncVendor.c +++ b/libgnucash/engine/gncVendor.c @@ -496,6 +496,8 @@ static void gncVendorFree (GncVendor *vendor) CACHE_REMOVE (vendor->notes); gncAddressBeginEdit (vendor->addr); gncAddressDestroy (vendor->addr); + + gncJobFreeList (vendor->jobs); g_list_free (vendor->jobs); g_free (vendor->balance); From 6a9ff287cddf1bee55c5d05331e0487372347151 Mon Sep 17 00:00:00 2001 From: Robert Fewell <14uBobIT@gmail.com> Date: Sat, 14 Aug 2021 11:01:38 +0100 Subject: [PATCH 3/9] Wrap BillTermDecRef and TaxTableDecRef in test for shutdown When Gnucash is shutting down, the TaxTables and BillTerms will be destroyed/freed and may already have been so there is no point trying to decrement a reference that is used to stop them being destroyed if in use. --- libgnucash/engine/gncCustomer.c | 9 +++++---- libgnucash/engine/gncEntry.c | 12 ++++++++---- libgnucash/engine/gncInvoice.c | 10 +++++++--- libgnucash/engine/gncVendor.c | 11 +++++++---- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/libgnucash/engine/gncCustomer.c b/libgnucash/engine/gncCustomer.c index 2ce39c8cce..676a7ed4cc 100644 --- a/libgnucash/engine/gncCustomer.c +++ b/libgnucash/engine/gncCustomer.c @@ -358,11 +358,12 @@ static void gncCustomerFree (GncCustomer *cust) g_list_free (cust->jobs); g_free (cust->balance); - if (cust->terms) - gncBillTermDecRef (cust->terms); - if (cust->taxtable) + if (!qof_book_shutting_down (qof_instance_get_book (QOF_INSTANCE(cust)))) { - gncTaxTableDecRef (cust->taxtable); + if (cust->terms) + gncBillTermDecRef (cust->terms); + if (cust->taxtable) + gncTaxTableDecRef (cust->taxtable); } /* qof_instance_release (&cust->inst); */ diff --git a/libgnucash/engine/gncEntry.c b/libgnucash/engine/gncEntry.c index b967544638..9d813d2e57 100644 --- a/libgnucash/engine/gncEntry.c +++ b/libgnucash/engine/gncEntry.c @@ -462,10 +462,14 @@ static void gncEntryFree (GncEntry *entry) gncAccountValueDestroy (entry->i_tax_values); if (entry->b_tax_values) gncAccountValueDestroy (entry->b_tax_values); - if (entry->i_tax_table) - gncTaxTableDecRef (entry->i_tax_table); - if (entry->b_tax_table) - gncTaxTableDecRef (entry->b_tax_table); + + if (!qof_book_shutting_down (qof_instance_get_book (QOF_INSTANCE(entry)))) + { + if (entry->i_tax_table) + gncTaxTableDecRef (entry->i_tax_table); + if (entry->b_tax_table) + gncTaxTableDecRef (entry->b_tax_table); + } /* qof_instance_release (&entry->inst); */ g_object_unref (entry); diff --git a/libgnucash/engine/gncInvoice.c b/libgnucash/engine/gncInvoice.c index 6ab155f813..ca9246136e 100644 --- a/libgnucash/engine/gncInvoice.c +++ b/libgnucash/engine/gncInvoice.c @@ -423,10 +423,14 @@ static void gncInvoiceFree (GncInvoice *invoice) g_list_free (invoice->entries); g_list_free (invoice->prices); - if (invoice->printname) g_free (invoice->printname); + if (invoice->printname) + g_free (invoice->printname); - if (invoice->terms) - gncBillTermDecRef (invoice->terms); + if (!qof_book_shutting_down (qof_instance_get_book (QOF_INSTANCE(invoice)))) + { + if (invoice->terms) + gncBillTermDecRef (invoice->terms); + } /* qof_instance_release (&invoice->inst); */ g_object_unref (invoice); diff --git a/libgnucash/engine/gncVendor.c b/libgnucash/engine/gncVendor.c index 50695de27d..1fa0a34100 100644 --- a/libgnucash/engine/gncVendor.c +++ b/libgnucash/engine/gncVendor.c @@ -501,10 +501,13 @@ static void gncVendorFree (GncVendor *vendor) g_list_free (vendor->jobs); g_free (vendor->balance); - if (vendor->terms) - gncBillTermDecRef (vendor->terms); - if (vendor->taxtable) - gncTaxTableDecRef (vendor->taxtable); + if (!qof_book_shutting_down (qof_instance_get_book (QOF_INSTANCE(vendor)))) + { + if (vendor->terms) + gncBillTermDecRef (vendor->terms); + if (vendor->taxtable) + gncTaxTableDecRef (vendor->taxtable); + } /* qof_instance_release (&vendor->inst); */ g_object_unref (vendor); From a9a3ed425a5fb8ce8da9ffd11fa380176f298176 Mon Sep 17 00:00:00 2001 From: Robert Fewell <14uBobIT@gmail.com> Date: Tue, 13 Jul 2021 15:10:35 +0100 Subject: [PATCH 4/9] Free Accounts on book close --- libgnucash/engine/Account.cpp | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/libgnucash/engine/Account.cpp b/libgnucash/engine/Account.cpp index ba09427581..6f09e63e2e 100644 --- a/libgnucash/engine/Account.cpp +++ b/libgnucash/engine/Account.cpp @@ -1316,7 +1316,7 @@ xaccFreeAccount (Account *acc) PERR (" instead of calling xaccFreeAccount(), please call\n" " xaccAccountBeginEdit(); xaccAccountDestroy();\n"); - /* First, recursively free children */ + /* First, recursively free children, also frees list */ xaccFreeAccountChildren(acc); } @@ -5991,14 +5991,32 @@ gnc_account_delete_all_bayes_maps (Account *acc) /* ================================================================ */ /* QofObject function implementation and registration */ +static void +destroy_all_child_accounts (Account *acc, gpointer data) +{ + xaccAccountBeginEdit (acc); + xaccAccountDestroy (acc); +} + static void gnc_account_book_end(QofBook* book) { - Account *root_account = gnc_book_get_root_account(book); + Account *root_account = gnc_book_get_root_account (book); + GList *accounts; + if (!root_account) return; - xaccAccountBeginEdit(root_account); - xaccAccountDestroy(root_account); + + accounts = gnc_account_get_descendants (root_account); + + if (accounts) + { + accounts = g_list_reverse (accounts); + g_list_foreach (accounts, (GFunc)destroy_all_child_accounts, nullptr); + g_list_free (accounts); + } + xaccAccountBeginEdit (root_account); + xaccAccountDestroy (root_account); } #ifdef _MSC_VER From a76fa5631c9b0abb8bdbe3d132ebe155b87d0a77 Mon Sep 17 00:00:00 2001 From: Robert Fewell <14uBobIT@gmail.com> Date: Tue, 13 Jul 2021 15:12:12 +0100 Subject: [PATCH 5/9] Free schedule Transaction accounts There is no need to act differently when destroying the scheduled transactions accounts, they can be destroyed when the scheduled transaction is freed under normal conditions and on book close. --- libgnucash/engine/SchedXaction.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/libgnucash/engine/SchedXaction.c b/libgnucash/engine/SchedXaction.c index ebbc1c48dd..51edb4e227 100644 --- a/libgnucash/engine/SchedXaction.c +++ b/libgnucash/engine/SchedXaction.c @@ -497,17 +497,8 @@ xaccSchedXactionFree( SchedXaction *sx ) delete_template_trans( sx ); - /* - * xaccAccountDestroy removes the account from - * its group for us AFAICT. If shutting down, - * the account is being deleted separately. - */ - - if (!qof_book_shutting_down(qof_instance_get_book(sx))) - { - xaccAccountBeginEdit(sx->template_acct); - xaccAccountDestroy(sx->template_acct); - } + xaccAccountBeginEdit( sx->template_acct ); + xaccAccountDestroy( sx->template_acct ); for ( l = sx->deferredList; l; l = l->next ) { From 77cf90f198c6cca27d74359ddbf989e64073c6e8 Mon Sep 17 00:00:00 2001 From: Robert Fewell <14uBobIT@gmail.com> Date: Tue, 13 Jul 2021 15:20:23 +0100 Subject: [PATCH 6/9] Free the template root account after the scheduled transactions have been freed. --- libgnucash/engine/SX-book.c | 6 ++++-- libgnucash/engine/SchedXaction.c | 10 +++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/libgnucash/engine/SX-book.c b/libgnucash/engine/SX-book.c index e15cb57f0b..19049753a5 100644 --- a/libgnucash/engine/SX-book.c +++ b/libgnucash/engine/SX-book.c @@ -127,7 +127,8 @@ sxtg_book_begin (QofBook *book) static void sxtg_book_end (QofBook *book) { -// gnc_book_set_template_root (book, NULL); + // Leave destroying template root account till after the scheduled + // transactions have been destroyed in gnc_sx_book_end. } static gboolean @@ -180,7 +181,7 @@ static QofObject sxtg_object_def = { DI(.interface_version = ) QOF_OBJECT_VERSION, DI(.e_type = ) GNC_ID_SXTG, - DI(.type_label = ) "Scheduled Transaction Templates", + DI(.type_label = ) "Scheduled Transaction Group", DI(.create = ) NULL, DI(.book_begin = ) sxtg_book_begin, DI(.book_end = ) sxtg_book_end, @@ -281,6 +282,7 @@ book_sxes_end(QofBook* book) sxes = qof_collection_get_data(col); if (sxes != NULL) { + g_list_free(sxes->sx_list); g_object_unref(sxes); qof_collection_set_data(col, NULL); } diff --git a/libgnucash/engine/SchedXaction.c b/libgnucash/engine/SchedXaction.c index 51edb4e227..255f4c6bb7 100644 --- a/libgnucash/engine/SchedXaction.c +++ b/libgnucash/engine/SchedXaction.c @@ -32,6 +32,7 @@ #include "Account.h" #include "SX-book.h" +#include "SX-book-p.h" #include "SX-ttinfo.h" #include "SchedXaction.h" #include "Transaction.h" @@ -510,7 +511,11 @@ xaccSchedXactionFree( SchedXaction *sx ) g_list_free( sx->deferredList ); sx->deferredList = NULL; } - + if ( sx->schedule ) + { + g_list_free( sx->schedule ); + sx->schedule = NULL; + } /* qof_instance_release (&sx->inst); */ g_object_unref( sx ); } @@ -1208,6 +1213,9 @@ gnc_sx_book_end(QofBook* book) col = qof_book_get_collection(book, GNC_ID_SCHEDXACTION); qof_collection_foreach(col, destroy_sx_on_book_close, NULL); + + // Now destroy the template root account + gnc_book_set_template_root (book, NULL); } #ifdef _MSC_VER From cacdb12aa5a5429af8dd0eac67ea9acca896e879 Mon Sep 17 00:00:00 2001 From: Robert Fewell <14uBobIT@gmail.com> Date: Fri, 16 Jul 2021 09:41:21 +0100 Subject: [PATCH 7/9] Dereference the prices used in function convert_price The two prices used in convert_price had there reference count increased in extract_common_prices. This lead to the reference count continuously increasing and not being freed on close so add gnc_price_unref on both prices to reduce the reference count. --- libgnucash/engine/gnc-pricedb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libgnucash/engine/gnc-pricedb.c b/libgnucash/engine/gnc-pricedb.c index 1b15a5e08b..996945fd81 100644 --- a/libgnucash/engine/gnc-pricedb.c +++ b/libgnucash/engine/gnc-pricedb.c @@ -2488,6 +2488,9 @@ convert_price (const gnc_commodity *from, const gnc_commodity *to, PriceTuple tu price = gnc_numeric_div (to_val, from_val, GNC_DENOM_AUTO, no_round); + gnc_price_unref (tuple.from); + gnc_price_unref (tuple.to); + if (from_cur == from && to_cur == to) return price; From 71d73beb6cd87fb0d4f59f3a3c1ae46867c73baf Mon Sep 17 00:00:00 2001 From: Robert Fewell <14uBobIT@gmail.com> Date: Sat, 17 Jul 2021 14:26:44 +0100 Subject: [PATCH 8/9] Original splits are not freed in xaccTransRollbackEdit At the beginning of a transactions edit, xaccTransBeginEdit is used and part of that duplicates the existing splits so that they can be restored if editing is cancelled. If cancelled, xaccTransRollbackEdit is used to restore the origin splits but if a split was not changed, the copy was not being freed so loop over the original split list and free them. --- libgnucash/engine/Transaction.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libgnucash/engine/Transaction.c b/libgnucash/engine/Transaction.c index 4890c2f015..f9ea4e684f 100644 --- a/libgnucash/engine/Transaction.c +++ b/libgnucash/engine/Transaction.c @@ -1769,7 +1769,7 @@ xaccTransRollbackEdit (Transaction *trans) xaccSplitRollbackEdit(s); SWAP_STR(s->action, so->action); SWAP_STR(s->memo, so->memo); - qof_instance_copy_kvp (QOF_INSTANCE (s), QOF_INSTANCE (so)); + qof_instance_copy_kvp (QOF_INSTANCE (s), QOF_INSTANCE (so)); s->reconciled = so->reconciled; s->amount = so->amount; s->value = so->value; @@ -1778,7 +1778,6 @@ xaccTransRollbackEdit (Transaction *trans) //SET_GAINS_A_VDIRTY(s); s->date_reconciled = so->date_reconciled; qof_instance_mark_clean(QOF_INSTANCE(s)); - xaccFreeSplit(so); } else { @@ -1805,6 +1804,10 @@ xaccTransRollbackEdit (Transaction *trans) } } g_list_free(slist); + + // orig->splits may still have duped splits so free them + for (node = orig->splits; node; node = node->next) + xaccFreeSplit(node->data); g_list_free(orig->splits); orig->splits = NULL; From 46f9fb01c972615ff77152f0c76489d761925e15 Mon Sep 17 00:00:00 2001 From: John Ralls Date: Tue, 24 Jan 2023 09:46:48 -0800 Subject: [PATCH 9/9] Simplify gnc_commodity equality and compare functions. gnc_commodity_compare is used for sorting and so needs to have a less-than return value. The only place it's used presents nothing to the UI; a GncGUID ordering is fast and stable. GnuCash allows only one commodity per book for any namespace/mnemonic pair, so the exhaustive string comparisons are superflous. While the current engine design allows only one instance of any object, meaning that a pointer comparison is sufficient to determine equality, that may not be true in the future, but the GncGUID is guaranteed to sufficiently identify a single commodity. Note that gnc_commodity_equiv is used to enforce that single-commodity rule and so cannot use GncGUID comparison. --- libgnucash/engine/gnc-commodity.c | 71 ++++++------------------------- 1 file changed, 12 insertions(+), 59 deletions(-) diff --git a/libgnucash/engine/gnc-commodity.c b/libgnucash/engine/gnc-commodity.c index ac686c4b7b..4d5ecea61d 100644 --- a/libgnucash/engine/gnc-commodity.c +++ b/libgnucash/engine/gnc-commodity.c @@ -38,6 +38,8 @@ #include "gnc-commodity.h" #include "gnc-locale-utils.h" #include "gnc-prefs.h" +#include "guid.h" +#include "qofinstance.h" static QofLogModule log_module = GNC_MOD_COMMODITY; @@ -1647,77 +1649,28 @@ gnc_commodity_equiv(const gnc_commodity * a, const gnc_commodity * b) priv_b = GET_PRIVATE(b); if (priv_a->name_space != priv_b->name_space) return FALSE; if (g_strcmp0(priv_a->mnemonic, priv_b->mnemonic) != 0) return FALSE; + return TRUE; } gboolean gnc_commodity_equal(const gnc_commodity * a, const gnc_commodity * b) { - gnc_commodityPrivate* priv_a; - gnc_commodityPrivate* priv_b; - gboolean same_book; - - if (a == b) return TRUE; - - if (!a || !b) - { - DEBUG ("one is NULL"); - return FALSE; - } - - priv_a = GET_PRIVATE(a); - priv_b = GET_PRIVATE(b); - same_book = qof_instance_get_book(QOF_INSTANCE(a)) == qof_instance_get_book(QOF_INSTANCE(b)); - - if ((same_book && priv_a->name_space != priv_b->name_space) - || (!same_book && g_strcmp0( gnc_commodity_namespace_get_name(priv_a->name_space), - gnc_commodity_namespace_get_name(priv_b->name_space)) != 0)) - { - DEBUG ("namespaces differ: %p(%s) vs %p(%s)", - priv_a->name_space, gnc_commodity_namespace_get_name(priv_a->name_space), - priv_b->name_space, gnc_commodity_namespace_get_name(priv_b->name_space)); - return FALSE; - } - - if (g_strcmp0(priv_a->mnemonic, priv_b->mnemonic) != 0) - { - DEBUG ("mnemonics differ: %s vs %s", priv_a->mnemonic, priv_b->mnemonic); - return FALSE; - } - - if (g_strcmp0(priv_a->fullname, priv_b->fullname) != 0) - { - DEBUG ("fullnames differ: %s vs %s", priv_a->fullname, priv_b->fullname); - return FALSE; - } - - if (g_strcmp0(priv_a->cusip, priv_b->cusip) != 0) - { - DEBUG ("cusips differ: %s vs %s", priv_a->cusip, priv_b->cusip); - return FALSE; - } - - if (priv_a->fraction != priv_b->fraction) - { - DEBUG ("fractions differ: %d vs %d", priv_a->fraction, priv_b->fraction); - return FALSE; - } - - return TRUE; + return gnc_commodity_compare(a, b) == 0; } +// Used as a sorting callback for deleting old prices, so it needs to be +// stable but doesn't need to be in any particular order sensible to humans. int gnc_commodity_compare(const gnc_commodity * a, const gnc_commodity * b) { - if (gnc_commodity_equal(a, b)) - { - return 0; - } - else - { - return 1; - } + if (a == b) return 0; + if (a && !b) return 1; + if (b && !a) return -1; + return qof_instance_guid_compare(a, b); } +// Used as a callback to g_list_find_custom, it should return 0 +// when the commodities match. int gnc_commodity_compare_void(const void * a, const void * b) { return gnc_commodity_compare(a, b);