From 55865d2e1b59214a6ea68927c9f58272b1837c38 Mon Sep 17 00:00:00 2001 From: Phil Longstaff Date: Sun, 9 Jan 2011 18:01:47 +0000 Subject: [PATCH] Fix more memory leaks. Also, when closing a book (to open a new one, not year end), free objects. git-svn-id: svn+ssh://svn.gnucash.org/repo/gnucash/trunk@20042 57a11ea4-9604-0410-9ed3-97b8803252fd --- src/engine/Account.c | 52 +++++++++++++++++++++++++++++---------- src/engine/Recurrence.c | 4 +-- src/engine/Recurrence.h | 2 +- src/engine/SX-book.c | 2 +- src/engine/SchedXaction.c | 38 +++++++++++++++++++++++++--- src/engine/Transaction.c | 39 ++++++++++++++++++++++++++--- src/engine/engine.i | 5 +++- src/engine/gnc-lot.c | 19 +++++++++++++- 8 files changed, 135 insertions(+), 26 deletions(-) diff --git a/src/engine/Account.c b/src/engine/Account.c index 4d1dbe7f7e..395e43b7cc 100644 --- a/src/engine/Account.c +++ b/src/engine/Account.c @@ -266,6 +266,10 @@ GList *gnc_account_list_name_violations (QofBook *book, const gchar *separator) else g_free ( acct_name ); } + if (accounts != NULL) + { + g_list_free(accounts); + } return invalid_list; } @@ -1230,13 +1234,26 @@ xaccAccountCommitEdit (Account *acc) PINFO ("freeing splits for account %p (%s)", acc, priv->accountName ? priv->accountName : "(null)"); - slist = g_list_copy(priv->splits); - for (lp = slist; lp; lp = lp->next) + book = qof_instance_get_book(acc); + + /* If book is shutting down, just clear the split list. The splits + themselves will be destroyed by the transaction code */ + if (!qof_book_shutting_down(book)) { - Split *s = lp->data; - xaccSplitDestroy (s); + slist = g_list_copy(priv->splits); + for (lp = slist; lp; lp = lp->next) + { + Split *s = lp->data; + xaccSplitDestroy (s); + } + g_list_free(slist); } - g_list_free(slist); + else + { + g_list_free(priv->splits); + priv->splits = NULL; + } + /* It turns out there's a case where this assertion does not hold: When the user tries to delete an Imbalance account, while also deleting all the splits in it. The splits will just get @@ -1245,18 +1262,17 @@ xaccAccountCommitEdit (Account *acc) g_assert(priv->splits == NULL || qof_book_shutting_down(acc->inst.book)); */ - book = qof_instance_get_book(acc); if (!qof_book_shutting_down(book)) { col = qof_book_get_collection(book, GNC_ID_TRANS); qof_collection_foreach(col, destroy_pending_splits_for_account, acc); - } - /* the lots should be empty by now */ - for (lp = priv->lots; lp; lp = lp->next) - { - GNCLot *lot = lp->data; - gnc_lot_destroy (lot); + /* the lots should be empty by now */ + for (lp = priv->lots; lp; lp = lp->next) + { + GNCLot *lot = lp->data; + gnc_lot_destroy (lot); + } } g_list_free(priv->lots); priv->lots = NULL; @@ -4807,6 +4823,16 @@ xaccAccountForEachTransaction(const Account *acc, TransactionCallback proc, /* ================================================================ */ /* QofObject function implementation and registration */ + +static void +gnc_account_book_end(QofBook* book) +{ + Account *root_account = gnc_book_get_root_account(book); + + xaccAccountBeginEdit(root_account); + xaccAccountDestroy(root_account); +} + #ifdef _MSC_VER /* MSVC compiler doesn't have C99 "designated initializers" * so we wrap them in a macro that is empty on MSVC. */ @@ -4821,7 +4847,7 @@ static QofObject account_object_def = DI(.type_label = ) "Account", DI(.create = ) (gpointer)xaccMallocAccount, DI(.book_begin = ) NULL, - DI(.book_end = ) NULL, + DI(.book_end = ) gnc_account_book_end, DI(.is_dirty = ) qof_collection_is_dirty, DI(.mark_clean = ) qof_collection_mark_clean, DI(.foreach = ) qof_collection_foreach, diff --git a/src/engine/Recurrence.c b/src/engine/Recurrence.c index 68a47bc08f..485e18620d 100644 --- a/src/engine/Recurrence.c +++ b/src/engine/Recurrence.c @@ -541,9 +541,9 @@ recurrenceListIsSemiMonthly(GList *recurrences) } gboolean -recurrenceListIsWeeklyMultiple(GList *recurrences) +recurrenceListIsWeeklyMultiple(const GList *recurrences) { - GList *r_iter; + const GList *r_iter; for (r_iter = recurrences; r_iter != NULL; r_iter = r_iter->next) { diff --git a/src/engine/Recurrence.h b/src/engine/Recurrence.h index dfbcc9b6f4..3c812d0148 100644 --- a/src/engine/Recurrence.h +++ b/src/engine/Recurrence.h @@ -161,7 +161,7 @@ gchar *recurrenceListToString(const GList *rlist); /** @return True if the recurrence list is a common "semi-monthly" recurrence. **/ gboolean recurrenceListIsSemiMonthly(GList *recurrences); /** @return True if the recurrence list is a common "weekly" recurrence. **/ -gboolean recurrenceListIsWeeklyMultiple(GList *recurrences); +gboolean recurrenceListIsWeeklyMultiple(const GList *recurrences); /** * Pretty-print an intentionally-short summary of the period of a (GList of) diff --git a/src/engine/SX-book.c b/src/engine/SX-book.c index 655a6b7da4..d32cb7d383 100644 --- a/src/engine/SX-book.c +++ b/src/engine/SX-book.c @@ -125,7 +125,7 @@ sxtg_book_begin (QofBook *book) static void sxtg_book_end (QofBook *book) { - gnc_book_set_template_root (book, NULL); +// gnc_book_set_template_root (book, NULL); } static gboolean diff --git a/src/engine/SchedXaction.c b/src/engine/SchedXaction.c index 4beaa45e92..c07cb90b6e 100644 --- a/src/engine/SchedXaction.c +++ b/src/engine/SchedXaction.c @@ -483,11 +483,15 @@ xaccSchedXactionFree( SchedXaction *sx ) /* * xaccAccountDestroy removes the account from - * its group for us AFAICT + * its group for us AFAICT. If shutting down, + * the account is being deleted separately. */ - xaccAccountBeginEdit(sx->template_acct); - xaccAccountDestroy(sx->template_acct); + if (!qof_book_shutting_down(qof_instance_get_book(sx))) + { + xaccAccountBeginEdit(sx->template_acct); + xaccAccountDestroy(sx->template_acct); + } for ( l = sx->deferredList; l; l = l->next ) { @@ -1248,6 +1252,9 @@ gnc_sx_remove_defer_instance( SchedXaction *sx, void *deferStateData ) * temporal-state-data instance list. The list should not be modified by the * caller; use the gnc_sx_{add,remove}_defer_instance() functions to modifiy * the list. + * + * @param sx Scheduled transaction + * @return Defer list which must not be modified by the caller **/ GList* gnc_sx_get_defer_instances( SchedXaction *sx ) @@ -1255,6 +1262,29 @@ gnc_sx_get_defer_instances( SchedXaction *sx ) return sx->deferredList; } +static void +destroy_sx_on_book_close(QofInstance *ent, gpointer data) +{ + SchedXaction* sx = GNC_SCHEDXACTION(ent); + + gnc_sx_begin_edit(sx); + xaccSchedXactionDestroy(sx); +} + +/** + * Destroys all SXes in the book because the book is being destroyed. + * + * @param book Book being destroyed + */ +static void +gnc_sx_book_end(QofBook* book) +{ + QofCollection *col; + + col = qof_book_get_collection(book, GNC_ID_SCHEDXACTION); + qof_collection_foreach(col, destroy_sx_on_book_close, NULL); +} + #ifdef _MSC_VER /* MSVC compiler doesn't have C99 "designated initializers" * so we wrap them in a macro that is empty on MSVC. */ @@ -1269,7 +1299,7 @@ static QofObject SXDesc = DI(.type_label = ) "Scheduled Transaction", DI(.create = ) (gpointer)xaccSchedXactionMalloc, DI(.book_begin = ) NULL, - DI(.book_end = ) NULL, + DI(.book_end = ) gnc_sx_book_end, DI(.is_dirty = ) qof_collection_is_dirty, DI(.mark_clean = ) qof_collection_mark_clean, DI(.foreach = ) qof_collection_foreach, diff --git a/src/engine/Transaction.c b/src/engine/Transaction.c index 76c8102c4f..d1e4a88048 100644 --- a/src/engine/Transaction.c +++ b/src/engine/Transaction.c @@ -1182,14 +1182,26 @@ do_destroy (Transaction *trans) qof_event_gen (&trans->inst, QOF_EVENT_DESTROY, NULL); - /* We only own the splits that still think they belong to us. */ - trans->splits = g_list_copy(trans->splits); + /* We only own the splits that still think they belong to us. This is done + in 2 steps. In the first, the splits are marked as being destroyed, but they + are not destroyed yet. In the second, the destruction is committed which will + do the actual destruction. If both steps are done for a split before they are + done for the next split, then a split will still be on the split list after it + has been freed. This can cause other parts of the code (e.g. in xaccSplitDestroy()) + to reference the split after it has been freed. */ for (node = trans->splits; node; node = node->next) { Split *s = node->data; if (s->parent == trans) { xaccSplitDestroy(s); + } + } + for (node = trans->splits; node; node = node->next) + { + Split *s = node->data; + if (s->parent == trans) + { xaccSplitCommitEdit(s); } } @@ -2296,6 +2308,27 @@ xaccTransFindSplitByAccount(const Transaction *trans, const Account *acc) \********************************************************************/ /* QofObject function implementation */ +static void +destroy_tx_on_book_close(QofInstance *ent, gpointer data) +{ + Transaction* tx = GNC_TRANSACTION(ent); + + xaccTransDestroy(tx); +} + +/** Handles book end - frees all transactions from the book + * + * @param book Book being closed + */ +static void +gnc_transaction_book_end(QofBook* book) +{ + QofCollection *col; + + col = qof_book_get_collection(book, GNC_ID_TRANS); + qof_collection_foreach(col, destroy_tx_on_book_close, NULL); +} + #ifdef _MSC_VER /* MSVC compiler doesn't have C99 "designated initializers" * so we wrap them in a macro that is empty on MSVC. */ @@ -2312,7 +2345,7 @@ static QofObject trans_object_def = DI(.type_label = ) "Transaction", DI(.create = ) (gpointer)xaccMallocTransaction, DI(.book_begin = ) NULL, - DI(.book_end = ) NULL, + DI(.book_end = ) gnc_transaction_book_end, DI(.is_dirty = ) qof_collection_is_dirty, DI(.mark_clean = ) qof_collection_mark_clean, DI(.foreach = ) qof_collection_foreach, diff --git a/src/engine/engine.i b/src/engine/engine.i index c4baefe523..e4c7151085 100644 --- a/src/engine/engine.i +++ b/src/engine/engine.i @@ -134,11 +134,14 @@ gchar * gnc_build_book_path (const gchar *filename); { SCM key_scm = SCM_CAR (path_scm); char *key; + gchar* gkey; if (!scm_is_string (key_scm)) break; - key = g_strdup (scm_to_locale_string (key_scm)); + key = scm_to_locale_string (key_scm); + gkey = g_strdup (key); + gnc_free_scm_locale_string(key); path = g_list_prepend (path, key); diff --git a/src/engine/gnc-lot.c b/src/engine/gnc-lot.c index fac90952b0..5ea828b338 100644 --- a/src/engine/gnc-lot.c +++ b/src/engine/gnc-lot.c @@ -614,6 +614,23 @@ gnc_lot_get_latest_split (GNCLot *lot) /* ============================================================= */ +static void +destroy_lot_on_book_close(QofInstance *ent, gpointer data) +{ + GNCLot* lot = GNC_LOT(ent); + + gnc_lot_destroy(lot); +} + +static void +gnc_lot_book_end(QofBook* book) +{ + QofCollection *col; + + col = qof_book_get_collection(book, GNC_ID_LOT); + qof_collection_foreach(col, destroy_lot_on_book_close, NULL); +} + #ifdef _MSC_VER /* MSVC compiler doesn't have C99 "designated initializers" * so we wrap them in a macro that is empty on MSVC. */ @@ -628,7 +645,7 @@ static QofObject gncLotDesc = DI(.type_label = ) "Lot", DI(.create = ) (gpointer)gnc_lot_new, DI(.book_begin = ) NULL, - DI(.book_end = ) NULL, + DI(.book_end = ) gnc_lot_book_end, DI(.is_dirty = ) qof_collection_is_dirty, DI(.mark_clean = ) qof_collection_mark_clean, DI(.foreach = ) qof_collection_foreach,