After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 347089 - Crash when editing transaction in two registers
Crash when editing transaction in two registers
Status: VERIFIED FIXED
Product: GnuCash
Classification: Other
Component: Register
2.0.x
Other Linux
: Normal critical
: ---
Assigned To: Chris Shoemaker
Chris Shoemaker
Depends on:
Blocks: 347575
 
 
Reported: 2006-07-10 12:45 UTC by Conrad Canterford
Modified: 2018-06-29 21:09 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Conrad Canterford 2006-07-10 12:45:08 UTC
** ERROR **: file split-register.c: line 832 (gnc_split_register_delete_current_split): assertion failed: (!xaccTransIsOpen(trans))

100% Replicatable. 

From one register (transaction journal view), start editing a transaction. Then, selecting a split that has another account as its target, click on the "Jump" toolbar button or select "Jump" from the right-click menu. In the new register, try to delete a split from the transaction. KaBOOM!
Comment 1 Andreas Köhler 2006-07-10 13:20:20 UTC
Confirmed.

Steps for an empty file:
* create accounts A and B
* open A (here in basic ledger mode)
* split button
* goto first splits account, choose A
* tab until back at date field
* goto second splits account, choose B
* tab until back at date field
* goto second split (account B), jump

* split button
* goto first split (account A)
* delete, confirm

bt:

  • #2 IA__g_assert_warning
  • #3 gnc_split_register_delete_current_split
    at split-register.c line 832
  • #4 gsr_default_delete_handler
    at gnc-split-reg.c line 1134
  • #5 gnc_plugin_page_register_cmd_delete_transaction
    at gnc-plugin-page-register.c line 2391

Comment 2 Chris Shoemaker 2006-07-12 03:15:40 UTC
Confirmed here, too.

I think this is one case of several where there were either crashers or silent data-loss bugs hiding before I added some asserts.

The problem is that if we allow the second register to open the transaction, then what happens if one register commits changes and the other does a rollback?  The second register must not be allowed to open the transaction, but I think I've made it more rigorous about checking these states, so I'll see if I can convince the second register to leave the transaction open.

*sigh* All these complications would disappear if the first register forced the pending transaction to be either committed or reverted when it lost focus.
Comment 3 Chris Shoemaker 2006-07-12 04:00:16 UTC
I tried:

1.  marking the transaction as pending in both registers, but making sure it was opened only once.  This leads to problems... the second register to close still has a pending transaction, but it's closed because the first register close it.  Ignoring that, all the code that assumes that the pending transaction is open breaks after the first register closes the transaction.


2.  deleting the split _without_ marking the transaction as pending.  This really terminally confuses the first register who was assuming the pending transaction wasn't going to become unanchored from that register.

The next thing I'll look into is the possibility of throwing an error dialog: "This transaction is currently being edited in another register.  Please finish that edit before beginning another."



Comment 4 Chris Shoemaker 2006-07-14 01:01:51 UTC
Hopefully fixed in r14495.
Comment 5 Andreas Köhler 2006-07-15 06:58:04 UTC
Needs backport. Adding to umbrella bug.
Comment 6 Derek Atkins 2006-07-15 15:42:53 UTC
Andreas, does comment #5 mean that you're audited the patch and verified it is correct?

Also, this patch does add a new (translated) string.  Are we backporting string changes to 2.0?
Comment 7 Derek Atkins 2006-07-15 16:24:47 UTC
Christian, what's your take on this?  In particular, what's your take on the "new string"?  I don't know if we have a policy about backporting string-changes to the release.
Comment 8 Christian Stimming 2006-07-16 13:22:46 UTC
From a translation point of view, I think it's fine to introduce new strings in 2.0 again. The only exception was the pre-release string freeze, but I'll post an updated string policy on that wiki page soon. Basically nothing is frozen anymore -- just go ahead.
Comment 9 Derek Atkins 2006-07-16 13:25:14 UTC
Well, I still think we should be cautious about new strings in 2.0 -- I don't think we should go about changing things willy nilly.  But thank you for the response.  I'll look at this patch now on other merits.
Comment 10 Derek Atkins 2006-07-16 17:17:05 UTC
I just looked at this patch and I have a few questions.  I sent my questions to -devel.  In particular, I'm wondering why the patch now calls xaccTransCommitEdit() outside of a gnc_suspend_gui_refresh() block.
Comment 11 Christian Stimming 2006-08-01 15:22:12 UTC
I wonder whether this went into 2.0.1 or is still under discussion.
Comment 12 Derek Atkins 2006-08-01 15:44:37 UTC
Sorry, it was not backported.  I'll try to provide an alternate approach that I think wont have as many issues, but I'm a bit busy at the moment with family issues.
Comment 13 Derek Atkins 2006-09-23 22:32:32 UTC
Okay, I think a combination of r14495 and r14887 should fix this in a way that I'm happy with.  Chris, if you want I can create a "patch branch" to pull these two changesets together for review.   If I get approval I'll backport both these changesets in a single changeset to the 2.0 branch.
Comment 14 Derek Atkins 2006-09-23 23:47:41 UTC
Approved by Chris.  Backported to 2.0 branch as r14889.
Comment 15 John Ralls 2018-06-29 21:09:11 UTC
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=347089. Please update any external references or bookmarks.