Created attachment 373893 [details] Simple test book with voided transaction in Accounts Receivable When performing a Check & Repair operation on an Accounts Receivable account which has a voided transaction, an infinite loop occurs. The trace file seems to say that gncScrubBusinessAccountSplits loops through the transactions in the account, calling gncScrubBusinessSplit on each split. After processing the voided transaction, gncScrubBusinessAccountSplits appears to restart its loop from the first transaction in the account, instead of continuing with the transaction following the voided transaction. The result is an infinite loop. To reproduce: 1. Run GnuCash 4.2. Bug is reproducible either with running the app from Finder, or running /Applications/Gnucash.app/Contents/MacOS/Gnucash --debug from Terminal. 2. Open attached book, Test Business GnC 4.2 2020-10a.gnucash. This is a new book with the Business Accounts tree, and just one transaction in the Accounts Receivable account, a voided deposit from Accounts Receivable to Checking Account. 3. Select the Accounts tab. 4. Select the Assets:Accounts Receivable account. 5. Actions… Check & Repair… Check & Repair Accounts Expected behaviour: 1. The Check & Repair completes quickly and without error Observed behaviour: 1. GnuCash hangs. 2. In the bottom-right corner of the main window, there will be a flickering message about "Checking business splits in the account Accounts Receivable: 0 of 1" 3. There appears to be no way to interrupt the hang. If GnuCash ran from terminal, a control-C stops it. If ran from the Finder, then one must Force Quit it. Discussion: The trace file (attached) has an repeating series of messages: * 00:06:57 INFO <gnc.engine.scrub> [gncScrubBusinessAccountSplits] Start processing split 1 of 1 * 00:06:57 INFO <gnc.engine.scrub> [gncScrubBusinessSplit] Destroying empty split 0x7fd1d20ff790 from transaction Sun Oct 18 03:59:00 2020 (Voided test transaction) * 00:06:57 INFO <gnc.engine.scrub> [gncScrubBusinessAccountSplits] Start processing split 1 of 1 * 00:06:57 INFO <gnc.engine.scrub> [gncScrubBusinessSplit] Destroying empty split 0x7fd1d20ff790 from transaction Sun Oct 18 03:59:00 2020 (Voided test transaction) This makes me think the hang is due to an infinite loop. I discovered this problem on my main book, which has 162 splits in an Accounts Receivable account. The status message alternated between "Checking business splits in the account Accounts Receivable: 0 of 162" and "Checking business splits in the account Accounts Receivable: 100 of 162". This reinforces my suspicion that this is an infinite loop caused by the loop restarting instead of continuing. Deleting the voided transactions from the Accounts Receivable account was sufficient to stop the problem from occurring. I can reproduce this bug with this test book and with my actual book 100% of the time. Check & Repair can process a voided transaction in a normal Checking Account with no problem, so I suspect the difficulty is related to the business features of the A/R account. The names "gncScrubBusinessAccountSplits" and "gncScrubBusinessSplit" in the trace file point to this also. I can capture a macOS execution sample if needed.
Created attachment 373894 [details] Trace file from Check & Repair on Accounts Receivable of test book
In contrast to Jean's comment in bug 797900 comment 9, I did not see what I would describe as visual feedback of the progress, which I would expect to be a progress bar with a cancel button. There are messages about what is currently being worked on in the lower-right corner of the main window. Maybe that's what Jean meant. Also, I saw no way to cancel the operation. If there is a progress bar with a cancel button, maybe it is hidden behind the main book window? That happens with various windows.
I took a quick look at the code, to see if I could get any clues as to cause. gncScrubBusinessAccountSplits() is at libgnucash/engine/ScrubBusiness.c:657..707. It does indeed loop over splits, calling gncScrubBusinessSplit(). This code and comment is at 695..698: // If gncScrubBusinessSplit returns true, a split was deleted and hence // The account split list has become invalid, so we need to start over if (gncScrubBusinessSplit (split)) goto restart; So it looks like the "restart the loop" part is intentional. gncScrubBusinessSplit() is at libgnucash/engine/ScrubBusiness.c:518..603. It has the following code for empty splits which aren't part of an invoice transaction. Voided transactions might well be included in this set. Maybe they shouldn't be. Lines 580..590 say (with some detail omitted): /* Next delete any empty splits that aren't part of an invoice transaction * Such splits may be the result of scrubbing the business lots, which can * merge splits together while reducing superfluous lot links */ else if (gnc_numeric_zero_p (xaccSplitGetAmount(split)) && !gncInvoiceGetInvoiceFromTxn (txn)) { …detail omitted… PINFO ("Destroying empty split %p from transaction %s (%s)", split, pdatestr, xaccTransGetDescription(txn)); xaccSplitDestroy (split); …detail omitted… deleted_split = TRUE; } …detail omitted… return deleted_split; So, if xaccSplitDestroy() is unable to delete a voided transaction, gncScrubBusinessSplit() won't know, and will return a deleted_split indication which causes the containing loop to restart, only to encounter the same voided transaction. xaccSplitDestroy() is at libgnucash/engine/ScrubBusiness.c:1451..1476. It appears to return a boolean. I suspect that TRUE means it thinks it destroyed the split, and FALSE means it thinks it didn't. gncScrubBusinessSplit() doesn't check the return value from xaccSplitDestroy(). Perhaps it should. I think that's all I can figure out from a quick look at the code. I hope this is helpful.
Thank you for the detailed report. The reason why the check&repair from the register works is that there is a test for the account type being AP or AR and the transaction split being associated with a 'lot', if not the gncScrubBusinessLot and gncScrubBusinessSplit are skipped. I think the simplest solution is to add is_void to the 'else if' statement above as that condition does not clear the transaction read only setting and is dealing with normal zero amount splits. I will add that to a PR with other changes in the area I was looking at.
I have added my change to PR803 just waiting for any feedback before pushing to maint.
I have pushed my change to maint and will be in the next nightly or version 4.3
