From 1020bde89c77f70cee6cc8181ead96e8fade47aa Mon Sep 17 00:00:00 2001 From: John Ralls Date: Tue, 14 Mar 2023 10:16:42 -0700 Subject: [PATCH] Fix crashes in test-engine on Arch Linux. Root cause is mutating a list while it's iterating. We're able to protect it in two cases, have to copy it in xaccTransScrubGainsDate. --- libgnucash/engine/Account.cpp | 6 ++++++ libgnucash/engine/Transaction.c | 4 +++- libgnucash/engine/test/utest-Transaction.cpp | 3 +++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/libgnucash/engine/Account.cpp b/libgnucash/engine/Account.cpp index 0e659e8d9f..036ef026d6 100644 --- a/libgnucash/engine/Account.cpp +++ b/libgnucash/engine/Account.cpp @@ -1342,6 +1342,12 @@ xaccFreeAccount (Account *acc) priv = GET_PRIVATE(acc); qof_event_gen (&acc->inst, QOF_EVENT_DESTROY, NULL); + /* Otherwise the lists below get munged while we're iterating + * them, possibly crashing. + */ + if (!qof_instance_get_destroying (acc)) + qof_instance_set_destroying(acc, TRUE); + if (priv->children) { PERR (" instead of calling xaccFreeAccount(), please call\n" diff --git a/libgnucash/engine/Transaction.c b/libgnucash/engine/Transaction.c index 1a77c4146e..edfadfe62d 100644 --- a/libgnucash/engine/Transaction.c +++ b/libgnucash/engine/Transaction.c @@ -2924,7 +2924,8 @@ static void xaccTransScrubGainsDate (Transaction *trans) { SplitList *node; - for (node = trans->splits; node; node = node->next) + SplitList *splits_copy = g_list_copy(trans->splits); + for (node = splits_copy; node; node = node->next) { Split *s = node->data; @@ -2943,6 +2944,7 @@ xaccTransScrubGainsDate (Transaction *trans) FOR_EACH_SPLIT(trans, s->gains &= ~GAINS_STATUS_DATE_DIRTY); } } + g_list_free(splits_copy); } /* ============================================================== */ diff --git a/libgnucash/engine/test/utest-Transaction.cpp b/libgnucash/engine/test/utest-Transaction.cpp index 878b5b6bcd..9bbfdba2d5 100644 --- a/libgnucash/engine/test/utest-Transaction.cpp +++ b/libgnucash/engine/test/utest-Transaction.cpp @@ -1427,6 +1427,9 @@ test_do_destroy (GainsFixture *fixture, gconstpointer pData) g_object_ref (base->txn); g_object_ref (fixture->gains_txn); + /* Protect against recursive calls to do_destroy from xaccTransCommitEdit */ + xaccTransBeginEdit(base->txn); + base->func->do_destroy (base->txn); g_assert_cmpint (test_signal_return_hits (sig), ==, 1); g_assert (base->txn->description == NULL);