Bug 316042 - KMyMoney hangs when editing transaction in anon file
Summary: KMyMoney hangs when editing transaction in anon file
Status: RESOLVED FIXED
Alias: None
Product: kmymoney
Classification: Applications
Component: general (show other bugs)
Version: 4.6.3
Platform: Microsoft Windows Microsoft Windows
: NOR critical
Target Milestone: ---
Assignee: KMyMoney Devel Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-02 22:25 UTC by toquehead
Modified: 2013-04-01 15:54 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Anon file to demonstrate problem. (3.57 MB, application/xml)
2013-03-02 22:35 UTC, toquehead
Details
Zipped anon file (250.18 KB, application/x-zip-compressed)
2013-03-03 05:42 UTC, toquehead
Details

Note You need to log in before you can comment on or make changes to this bug.
Description toquehead 2013-03-02 22:25:29 UTC
kmy hangs editing transaction with high cpu use.

Reproducible: Always

Steps to Reproduce:
1. Open ledger for account A000632.
2. Go to transaction dated 31/12/12 for security E00041.
3. Click to edit the transaction. 

Actual Results:  
kmy hangs with high cpu use.

Expected Results:  
edit the transaction

I am on Windows 7. I encountered this _only_ in an anon file I created for the purpose of reporting a different bug. The first time I tried to create the anon file, my Windows 7 system went down hard.
Comment 1 toquehead 2013-03-02 22:35:56 UTC
Created attachment 77694 [details]
Anon file to demonstrate problem.
Comment 2 toquehead 2013-03-02 22:46:49 UTC
I have confirmed the same behaviour (kmy 4.6.2) under Mint 11 running in a VMWare box.
Comment 3 allan 2013-03-02 23:42:25 UTC
I'm unable to open the attachment xml file because of 'error on line 18258 at column 8: Char 0x0 out of allowed range'.
Could you perhaps try again, but using Mint.  If so, open using Kate or KWrite.  I don't think gedit will do this.  If it appears OK, either totally, or to line 18258, send it to me off-line.
Comment 4 allan 2013-03-03 00:09:11 UTC
(In reply to comment #3)
> I'm unable to open the attachment xml file because of 'error on line 18258
> at column 8: Char 0x0 out of allowed range'.
> Could you perhaps try again, but using Mint.  If so, open using Kate or
> KWrite.  I don't think gedit will do this.  If it appears OK, either
> totally, or to line 18258, send it to me off-line.

I was able to view the file by dragging the attachment into the open window of Kate.  It appears OK up to line 18258 (2009), but from there on, it is corrupted.
From your Steps to Reproduce:
1. Open ledger for account A000632.
2. Go to transaction dated 31/12/12 for security E00041.
3. Click to edit the transaction.
it sounds like that file could load, so what's different between that file and the one you sent?
Comment 5 toquehead 2013-03-03 05:42:14 UTC
Created attachment 77708 [details]
Zipped anon file
Comment 6 allan 2013-03-03 12:38:50 UTC
(In reply to comment #5)
> Created attachment 77708 [details]
> Zipped anon file

OK, that's better.  
Looking at the xml file, so far, I have not seen anything amiss.
Loading it into KMM, and opening A000632, while everything looks normal, so far, there is a definite problem, not just with that one transaction, but every transaction in every sub-account, cannot be edited without KMM locking up.  I'll continue looking, but I might need a seasoned eye to assist.
One thing I have noticed, though, is that the particular transaction mentioned can be moved to another account, where it behaves OK.  In fact, after moving all transactions from that account, they all seem OK in their new home.  Trying now to enter a new transaction into the now empty account causes a lockup again.
Comment 7 allan 2013-03-03 17:27:31 UTC
Account A000629 also has the problem.

Can we backtrack.  The problem you reported originally was that for a particular transaction, you were unable to edit the fees/interest field, but other fields were OK.  Since then, the problem appears to have spread.  Can you think of any possible cause?  Is that file still available?
Comment 8 toquehead 2013-03-03 22:31:24 UTC
I still have my original kmy file, and it is still behaves the same way: I can't edit 2 fields in an transaction, but I do NOT see the blocking/hanging behaviour that we are seeing in the anonized file. So, it seems to me to be related to the anonymizing process, but we can't really tackle my original problem until I can send you a working anon file.
Comment 9 allan 2013-03-04 00:35:25 UTC
I haven't digested this yet, but there is progress, of sorts.

It seems to go wrong in InvestTransactionEditor::updatePriceMode, where it is checking the price mode (price per share or total amount).  So I edited one of the securities, which was set to 'default', and changed it to 'price per share'.  Surprise, surprise - no hang now, at least on that security.

So, now the anon file behaves as originally described - editing of the fee field is ignored.
Comment 10 allan 2013-03-04 12:05:59 UTC
I think we have two separate problems here  - The first being editing of the fee field is being ignored, and the second is the anon.xml file looping, and think it would be as well to separate them.
For now, the looping occurs in the while() loop in InvestTransactionEditor::priceModeE InvestTransactionEditor::priceMode(void) const.

while (!accId.isEmpty() && mode == 0) {
    MyMoneyAccount acc = MyMoneyFile::instance()->account(accId);
    if (acc.value("priceMode").isEmpty()) 
      accId = acc.parentAccountId();
    else
      mode = static_cast<priceModeE>(acc.value("priceMode").toInt());
  }

Initially, acc.value("priceMode").isEmpty() is true and the parent (the investment) account id gets used.  In the parent account, <PAIR key="priceMode" value="x"/>, and mode equates to '0' so the loop continues.

How does the parent account, <PAIR key="priceMode" get set by the user?  It's in the xml file.
Comment 11 allan 2013-03-05 20:39:12 UTC
(In reply to comment #5)
> Created attachment 77708 [details]
> Zipped anon file

Several accounts in this file have an entry <PAIR key="priceMode" value="x"/>, in particular the one now called A000632.

Could you check for the presence of <PAIR key="priceMode" in the corresponding account, and elsewhere, in the non-anon file.  What if anything does it show?
Comment 12 toquehead 2013-03-05 20:59:28 UTC
I get 5 hits. Two accounts and three investments. I assume this is the "price entry" setting in kmy. All other accounts and investments must be "default" and not stored.

I'm not totally sure why I changed that setting for the accounts, but for investments like money market funds that have a fixed price, I needed to change that to do data entry in a sensible way.

I have gone back and reverted some of those to default to see if it affected the problem, but it didn't seem to.
Comment 13 allan 2013-03-05 21:55:56 UTC
(In reply to comment #12)
> I get 5 hits. Two accounts and three investments. I assume this is the
> "price entry" setting in kmy. All other accounts and investments must be
> "default" and not stored.
> 
> I'm not totally sure why I changed that setting for the accounts, but for
> investments like money market funds that have a fixed price, I needed to
> change that to do data entry in a sensible way.
> 
> I have gone back and reverted some of those to default to see if it affected
> the problem, but it didn't seem to.

Sorry, I didn't make myself clear.  What I was looking for was the whole of the actual entry, but in particular, does 'value="x" ' actually appear, or what?

Do I take it that the actual original settings you had were price per share?  I think the ones that were changed are the ones that cause the looping with the anon file.
Comment 14 toquehead 2013-03-05 22:28:15 UTC
Ah, OK. I thought you were using 'x' as a placeholder. In the original:

(4008):    <PAIR key="priceMode" value="2"/>
(4026):    <PAIR key="priceMode" value="2"/>
(4046):    <PAIR key="priceMode" value="1"/>
(4431):    <PAIR key="priceMode" value="1"/>
(4824):    <PAIR key="priceMode" value="1"/>

2 == total for all shares
1 == price per share

I could try to track down what the x's should be in the anon file, but it probably doesn't matter for our purposes. Just change the x's to 1's, or I'm guessing you could delete those lines and revert to default.
Comment 15 allan 2013-03-05 22:39:36 UTC
(In reply to comment #14)
> Ah, OK. I thought you were using 'x' as a placeholder. In the original:
> 
> (4008):    <PAIR key="priceMode" value="2"/>
> (4026):    <PAIR key="priceMode" value="2"/>
> (4046):    <PAIR key="priceMode" value="1"/>
> (4431):    <PAIR key="priceMode" value="1"/>
> (4824):    <PAIR key="priceMode" value="1"/>
> 
> 2 == total for all shares
> 1 == price per share
> 
> I could try to track down what the x's should be in the anon file, but it
> probably doesn't matter for our purposes. Just change the x's to 1's, or I'm
> guessing you could delete those lines and revert to default.

I have in fact just tried changing them all to value="1" and those accounts are now editable without causing the looping.

I suspect that the basic problem is that the anonymising is entering the "x", but KMM is not expecting a non-numeric.

I could do with an opinion from the devs as to whether to change the anon process (which I guess is the proper solution), or to change the code where the looping occurs.
Comment 16 allan 2013-03-26 23:37:34 UTC
(In reply to comment #15)
> (In reply to comment #14)
> > Ah, OK. I thought you were using 'x' as a placeholder. In the original:
> > 
> > (4008):    <PAIR key="priceMode" value="2"/>
> > (4026):    <PAIR key="priceMode" value="2"/>
> > (4046):    <PAIR key="priceMode" value="1"/>
> > (4431):    <PAIR key="priceMode" value="1"/>
> > (4824):    <PAIR key="priceMode" value="1"/>
> > 
> > 2 == total for all shares
> > 1 == price per share
> > 
<snip>
> I suspect that the basic problem is that the anonymising is entering the
> "x", but KMM is not expecting a non-numeric.
> 
> I could do with an opinion from the devs as to whether to change the anon
> process (which I guess is the proper solution), or to change the code where
> the looping occurs.

@devs.  Any thoughts, please?
Comment 17 Thomas Baumgart 2013-03-29 08:09:59 UTC
Yes, it looks like this is the cause to the looping problem.

Add 'priceMode' to the MyMoneyStorageANON::zKvpNoModify QStringList at the beginning of mymoneystorageanon.cpp and the problem of the looping should be solved for the next save of an anon file. Please feel free to add this after testing to git master.
Comment 18 allan 2013-03-29 22:32:39 UTC
Git commit 240289fa0c3391dd71d0c13a44a43fa6fd8deaf7 by Allan Anderson.
Committed on 29/03/2013 at 23:02.
Pushed by allananderson into branch 'master'.
Fix problem when creating an Anon file, where a priceMode setting in an
investment or a stock account, was anonymized, resulting in looping
in InvestTransactionEditor::priceMode(void) on attempting to
edit the account in question.

M  +1    -1    kmymoney/mymoney/storage/mymoneystorageanon.cpp

http://commits.kde.org/kmymoney/240289fa0c3391dd71d0c13a44a43fa6fd8deaf7
Comment 19 Cristian Oneț 2013-04-01 15:54:50 UTC
Git commit d77a39f3f09993fe33ecd7bf75a41018ca982a8f by Cristian Oneț, on behalf of Allan Anderson.
Committed on 29/03/2013 at 23:02.
Pushed by conet into branch '4.6'.
Fix problem when creating an Anon file, where a priceMode setting in an
investment or a stock account, was anonymized, resulting in looping
in InvestTransactionEditor::priceMode(void) on attempting to
edit the account in question.
(cherry picked from commit 240289fa0c3391dd71d0c13a44a43fa6fd8deaf7)

M  +1    -1    kmymoney/mymoney/storage/mymoneystorageanon.cpp

http://commits.kde.org/kmymoney/d77a39f3f09993fe33ecd7bf75a41018ca982a8f